From 7c2a6e793f30e94454eddceed75e310e507ce317 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Thu, 3 Aug 2023 14:52:53 +0200 Subject: [PATCH 1/2] fix: Harden outbox/draft message retrieval of shared storage Signed-off-by: Christoph Wurst --- lib/Db/LocalMessageMapper.php | 6 ++++-- lib/Service/DraftsService.php | 2 +- lib/Service/OutboxService.php | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/Db/LocalMessageMapper.php b/lib/Db/LocalMessageMapper.php index 5558692607..fd8db38586 100644 --- a/lib/Db/LocalMessageMapper.php +++ b/lib/Db/LocalMessageMapper.php @@ -100,16 +100,18 @@ public function getAllForUser(string $userId, int $type = LocalMessage::TYPE_OUT } /** + * @param LocalMessage::TYPE_* $type * @throws DoesNotExistException */ - public function findById(int $id, string $userId): LocalMessage { + public function findById(int $id, string $userId, int $type): LocalMessage { $qb = $this->db->getQueryBuilder(); $qb->select('m.*') ->from('mail_accounts', 'a') ->join('a', $this->getTableName(), 'm', $qb->expr()->eq('m.account_id', 'a.id')) ->where( $qb->expr()->eq('a.user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR), IQueryBuilder::PARAM_STR), - $qb->expr()->eq('m.id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT) + $qb->expr()->eq('m.id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT), + $qb->expr()->eq('type', $qb->createNamedParameter($type, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT), ); $entity = $this->findEntity($qb); $entity->setAttachments($this->attachmentMapper->findByLocalMessageId($userId, $id)); diff --git a/lib/Service/DraftsService.php b/lib/Service/DraftsService.php index 2efbf51ee7..2d7f996471 100644 --- a/lib/Service/DraftsService.php +++ b/lib/Service/DraftsService.php @@ -208,7 +208,7 @@ public function sendMessage(LocalMessage $message, Account $account): void { * @throws DoesNotExistException */ public function getMessage(int $id, string $userId): LocalMessage { - return $this->mapper->findById($id, $userId); + return $this->mapper->findById($id, $userId, LocalMessage::TYPE_DRAFT); } /** diff --git a/lib/Service/OutboxService.php b/lib/Service/OutboxService.php index 46064176a5..7512585ebf 100644 --- a/lib/Service/OutboxService.php +++ b/lib/Service/OutboxService.php @@ -117,7 +117,7 @@ public function getMessages(string $userId): array { * @throws DoesNotExistException */ public function getMessage(int $id, string $userId): LocalMessage { - return $this->mapper->findById($id, $userId); + return $this->mapper->findById($id, $userId, LocalMessage::TYPE_OUTGOING); } /** From 20aec7421e8c0cac8659f3bb03e66addae4bf138 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Thu, 3 Aug 2023 16:14:32 +0200 Subject: [PATCH 2/2] fix: Convert drafts to outbox messages before sending Signed-off-by: Christoph Wurst --- appinfo/routes.php | 5 +++++ lib/Controller/OutboxController.php | 20 +++++++++++++++++++ lib/Service/OutboxService.php | 7 +++++++ src/components/NewMessageModal.vue | 15 +++++++++++--- src/service/DraftService.js | 6 ++---- src/service/OutboxService.js | 10 ++++++++++ src/store/outbox/actions.js | 13 ++++++++++++ .../Integration/Db/LocalMessageMapperTest.php | 4 ++-- tests/Integration/Db/RecipientMapperTest.php | 2 +- 9 files changed, 72 insertions(+), 10 deletions(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index 43b206baa1..5157779146 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -365,6 +365,11 @@ 'url' => '/api/outbox/{id}', 'verb' => 'POST' ], + [ + 'name' => 'outbox#createFromDraft', + 'url' => '/api/outbox/from-draft/{id}', + 'verb' => 'POST' + ], [ 'name' => 'googleIntegration#configure', 'url' => '/api/integration/google', diff --git a/lib/Controller/OutboxController.php b/lib/Controller/OutboxController.php index 738e274581..3f29705d17 100644 --- a/lib/Controller/OutboxController.php +++ b/lib/Controller/OutboxController.php @@ -32,6 +32,7 @@ use OCA\Mail\Http\JsonResponse; use OCA\Mail\Http\TrapError; use OCA\Mail\Service\AccountService; +use OCA\Mail\Service\DraftsService; use OCA\Mail\Service\OutboxService; use OCA\Mail\Service\SmimeService; use OCP\AppFramework\Controller; @@ -149,6 +150,25 @@ public function create( return JsonResponse::success($message, Http::STATUS_CREATED); } + /** + * @NoAdminRequired + * + * @return JsonResponse + */ + #[TrapError] + public function createFromDraft(DraftsService $draftsService, int $id, int $sendAt = null): JsonResponse { + $draftMessage = $draftsService->getMessage($id, $this->userId); + // Locate the account to check authorization + $this->accountService->find($this->userId, $draftMessage->getAccountId()); + + $outboxMessage = $this->service->convertDraft($draftMessage, $sendAt); + + return JsonResponse::success( + $outboxMessage, + Http::STATUS_CREATED, + ); + } + /** * @NoAdminRequired * diff --git a/lib/Service/OutboxService.php b/lib/Service/OutboxService.php index 7512585ebf..e112e714dd 100644 --- a/lib/Service/OutboxService.php +++ b/lib/Service/OutboxService.php @@ -274,4 +274,11 @@ public function flush(): void { } } } + + public function convertDraft(LocalMessage $draftMessage, int $sendAt): LocalMessage { + $outboxMessage = clone $draftMessage; + $outboxMessage->setType(LocalMessage::TYPE_OUTGOING); + $outboxMessage->setSendAt($sendAt); + return $this->mapper->update($outboxMessage); + } } diff --git a/src/components/NewMessageModal.vue b/src/components/NewMessageModal.vue index edba359d25..59655ab72a 100644 --- a/src/components/NewMessageModal.vue +++ b/src/components/NewMessageModal.vue @@ -212,7 +212,7 @@ export default { let idToReturn const dataForServer = this.getDataForServer(data, true) if (!id) { - const { id } = await saveDraft(data.account, dataForServer) + const { id } = await saveDraft(dataForServer) dataForServer.id = id await this.$store.dispatch('patchComposerData', { id, draftId: dataForServer.draftId }) this.canSaveDraft = true @@ -307,17 +307,26 @@ export default { } if (!this.composerData.id) { - const { id } = await saveDraft(data.account, dataForServer) + // This is a new message + const { id } = await saveDraft(dataForServer) dataForServer.id = id await this.$store.dispatch('outbox/enqueueMessage', { message: dataForServer, }) - } else { + } else if (this.composerData.type === 0) { + // This is an outbox message dataForServer.id = this.composerData.id await this.$store.dispatch('outbox/updateMessage', { message: dataForServer, id: this.composerData.id, }) + } else { + // This is a draft + dataForServer.id = this.composerData.id + await this.$store.dispatch('outbox/enqueueFromDraft', { + draftMessage: dataForServer, + id: this.composerData.id, + }) } if (!data.sendAt || data.sendAt < Math.floor((now + UNDO_DELAY) / 1000)) { diff --git a/src/service/DraftService.js b/src/service/DraftService.js index 0f01d134dd..7327ef0e1e 100644 --- a/src/service/DraftService.js +++ b/src/service/DraftService.js @@ -25,10 +25,8 @@ import { generateUrl } from '@nextcloud/router' import { convertAxiosError } from '../errors/convert' -export async function saveDraft(accountId, data) { - const url = generateUrl('/apps/mail/api/drafts', { - accountId, - }) +export async function saveDraft(data) { + const url = generateUrl('/apps/mail/api/drafts') try { return (await axios.post(url, data)).data.data diff --git a/src/service/OutboxService.js b/src/service/OutboxService.js index 0f89297cbc..11b2f44baf 100644 --- a/src/service/OutboxService.js +++ b/src/service/OutboxService.js @@ -45,6 +45,16 @@ export async function enqueueMessage(message) { const { data } = await axios.post(url, message) return data.data } + +export async function enqueueMessageFromDraft(id, message) { + const url = generateUrl('/apps/mail/api/outbox/from-draft/{id}', { + id, + }) + + const { data } = await axios.post(url, message) + return data.data +} + export async function updateMessage(message, id) { const url = generateUrl('/apps/mail/api/outbox/{id}', { id, diff --git a/src/store/outbox/actions.js b/src/store/outbox/actions.js index ff963ab101..5d082f54a3 100644 --- a/src/store/outbox/actions.js +++ b/src/store/outbox/actions.js @@ -73,6 +73,19 @@ export default { return message }, + async enqueueFromDraft({ commit }, { id, draftMessage }) { + const message = await OutboxService.enqueueMessageFromDraft(id, draftMessage) + + commit('addMessage', { message }) + + // Future drafts/sends after an error should go through outbox logic + commit('convertComposerMessageToOutbox', { message }, { + root: true, + }) + + return message + }, + async stopMessage({ commit }, { message }) { commit('stopMessage', { message }) const updatedMessage = await OutboxService.updateMessage({ diff --git a/tests/Integration/Db/LocalMessageMapperTest.php b/tests/Integration/Db/LocalMessageMapperTest.php index 22b24d010c..e5505236b0 100644 --- a/tests/Integration/Db/LocalMessageMapperTest.php +++ b/tests/Integration/Db/LocalMessageMapperTest.php @@ -107,7 +107,7 @@ public function testFindAllForUser(): void { * @depends testFindAllForUser */ public function testFindById(): void { - $row = $this->mapper->findById($this->entity->getId(), $this->account->getUserId()); + $row = $this->mapper->findById($this->entity->getId(), $this->account->getUserId(), LocalMessage::TYPE_OUTGOING); $this->assertEquals(LocalMessage::TYPE_OUTGOING, $row->getType()); $this->assertEquals(2, $row->getAliasId()); @@ -122,7 +122,7 @@ public function testFindById(): void { public function testFindByIdNotFound(): void { $this->expectException(DoesNotExistException::class); - $this->mapper->findById(1337, $this->account->getUserId()); + $this->mapper->findById(1337, $this->account->getUserId(), LocalMessage::TYPE_DRAFT); } /** diff --git a/tests/Integration/Db/RecipientMapperTest.php b/tests/Integration/Db/RecipientMapperTest.php index fa6eead15e..eb4720b5a0 100644 --- a/tests/Integration/Db/RecipientMapperTest.php +++ b/tests/Integration/Db/RecipientMapperTest.php @@ -190,7 +190,7 @@ public function testUpdateRecipients(): void { $results = $this->mapper->findByLocalMessageId($message->getId()); $this->assertCount(1, $results); - $message = $this->localMessageMapper->findById($message->getId(), $this->getTestAccountUserId()); + $message = $this->localMessageMapper->findById($message->getId(), $this->getTestAccountUserId(), LocalMessage::TYPE_OUTGOING); $pierre = new Recipient(); $pierre->setLabel('Pierre');