fixup! feat(ocs): send a message via api

nextcloud · Jun 26, 2024 · b8b60dc · b8b60dc
1 parent 3856957
commit b8b60dc
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 28 deletions.
diff --git a/lib/Controller/MessageApiController.php b/lib/Controller/MessageApiController.php
@@ -34,7 +34,7 @@ class MessageApiController extends OCSController {
 
 	public function __construct(
 		string $appName,
-		$UserId,
+		?string $userId,
 		IRequest $request,
 		private AccountService $accountService,
 		private AliasesService $aliasesService,
@@ -44,21 +44,9 @@ public function __construct(
 		private ITimeFactory $time,
 	) {
 		parent::__construct($appName, $request);
-		$this->userId = $UserId;
+		$this->userId = $userId;
 	}
 
-	/**
-	 * @param int $accountId
-	 * @param string $fromEmail
-	 * @param string $subject
-	 * @param string $body
-	 * @param bool $isHtml
-	 * @param array $to
-	 * @param array $cc
-	 * @param array $bcc
-	 * @param array $references
-	 * @return DataResponse
-	 */
 	#[UserRateLimit(limit: 5, period: 100)]
 	#[NoAdminRequired]
 	#[NoCSRFRequired]
@@ -73,6 +61,10 @@ public function send(
 		array $bcc = [],
 		array $references = [],
 	): DataResponse {
+		if($this->userId === null) {
+			return new DataResponse('', Http::STATUS_FORBIDDEN);
+		}
+
 		try {
 			$mailAccount = $this->accountService->find($this->userId, $accountId);
 		} catch (ClientException $e) {
@@ -82,7 +74,7 @@ public function send(
 
 		if($fromEmail !== $mailAccount->getEmail()) {
 			try {
-				$alias = $this->aliasesService->findByAliasEmailAndUserId($fromEmail, $this->userId);
+				$alias = $this->aliasesService->findByAliasAndUserId($fromEmail, $this->userId);
 			} catch (DoesNotExistException $e) {
 				$this->logger->error("Alias $fromEmail for mail account $accountId not found", ['exception' => $e]);
 				// Cannot send from this email as it is not configured as an alias
@@ -156,7 +148,7 @@ public function send(
 
 		return match ($localMessage->getStatus()) {
 			LocalMessage::STATUS_PROCESSED => new DataResponse('Success', Http::STATUS_OK),
-			LocalMessage::STATUS_NO_SENT_MAILBOX => new DataResponse('Configuration errror: Cannot send message without sent mailbox.', Http::STATUS_FORBIDDEN),
+			LocalMessage::STATUS_NO_SENT_MAILBOX => new DataResponse('Configuration error: Cannot send message without sent mailbox.', Http::STATUS_FORBIDDEN),
 			LocalMessage::STATUS_SMPT_SEND_FAIL => new DataResponse('An SMTP error occured, please check your mail server logs.', Http::STATUS_INTERNAL_SERVER_ERROR),
 			LocalMessage::STATUS_IMAP_SENT_MAILBOX_FAIL => new DataResponse('Email was sent but could not be copied to sent mailbox.', Http::STATUS_ACCEPTED),
 			default => new DataResponse('An error occured, please check the logs.', Http::STATUS_INTERNAL_SERVER_ERROR),

diff --git a/lib/Service/AliasesService.php b/lib/Service/AliasesService.php
@@ -67,9 +67,8 @@ public function find(int $aliasId, string $currentUserId): Alias {
 	 * @return Alias
 	 * @throws DoesNotExistException
 	 */
-	public function findByAliasEmailAndUserId(string $aliasEmail, string $userId): Alias {
+	public function findByAliasAndUserId(string $aliasEmail, string $userId): Alias {
 		return $this->aliasMapper->findByAlias($aliasEmail, $userId);
-
 	}
 
 	/**

diff --git a/tests/Unit/Controller/MessageApiControllerTest.php b/tests/Unit/Controller/MessageApiControllerTest.php
@@ -98,7 +98,7 @@ public function testSend($messageStatus, $expected): void {
 			->with($this->userId, $this->account->getId())
 			->willReturn($this->account);
 		$this->aliasesService->expects(self::never())
-			->method('findByAliasEmailAndUserId');
+			->method('findByAliasAndUserId');
 		$this->request->expects(self::once())
 			->method('getUploadedFile')
 			->willReturn(null);
@@ -127,7 +127,7 @@ public function testSend($messageStatus, $expected): void {
 	public function mailData() {
 		return [
 			[LocalMessage::STATUS_PROCESSED, new DataResponse('Success', Http::STATUS_OK)],
-			[LocalMessage::STATUS_NO_SENT_MAILBOX, new DataResponse('Configuration errror: Cannot send message without sent mailbox.', Http::STATUS_FORBIDDEN)],
+			[LocalMessage::STATUS_NO_SENT_MAILBOX, new DataResponse('Configuration error: Cannot send message without sent mailbox.', Http::STATUS_FORBIDDEN)],
 			[LocalMessage::STATUS_SMPT_SEND_FAIL, new DataResponse('An SMTP error occured, please check your mail server logs.', Http::STATUS_INTERNAL_SERVER_ERROR)],
 			[LocalMessage::STATUS_IMAP_SENT_MAILBOX_FAIL, new DataResponse('Email was sent but could not be copied to sent mailbox.', Http::STATUS_ACCEPTED)],
 			[LocalMessage::STATUS_TOO_MANY_RECIPIENTS, new DataResponse('An error occured, please check the logs.', Http::STATUS_INTERNAL_SERVER_ERROR)],
@@ -143,7 +143,7 @@ public function testSendException($exception, $expected): void {
 			->with($this->userId, $this->account->getId())
 			->willReturn($this->account);
 		$this->aliasesService->expects(self::never())
-			->method('findByAliasEmailAndUserId');
+			->method('findByAliasAndUserId');
 		$this->request->expects(self::once())
 			->method('getUploadedFile')
 			->willReturn(null);
@@ -208,7 +208,7 @@ public function testWithAttachment(): void {
 			->with($this->userId, $this->account->getId())
 			->willReturn($this->account);
 		$this->aliasesService->expects(self::never())
-			->method('findByAliasEmailAndUserId');
+			->method('findByAliasAndUserId');
 		$this->request->expects(self::once())
 			->method('getUploadedFile')
 			->willReturn($attachments);
@@ -223,7 +223,7 @@ public function testWithAttachment(): void {
 			->method('sendMessage')
 			->willReturn($this->message);
 
-		$expected = new DataResponse(null, Http::STATUS_OK);
+		$expected = new DataResponse('Success', Http::STATUS_OK);
 		$actual = $this->controller->send(
 			$this->accountId,
 			$this->fromEmail,
@@ -265,7 +265,7 @@ public function testWithAttachmentError(): void {
 			->with($this->userId, $this->account->getId())
 			->willReturn($this->account);
 		$this->aliasesService->expects(self::never())
-			->method('findByAliasEmailAndUserId');
+			->method('findByAliasAndUserId');
 		$this->request->expects(self::once())
 			->method('getUploadedFile')
 			->willReturn($attachments);
@@ -313,7 +313,7 @@ public function testAlias(): void {
 			->with($this->userId, $account->getId())
 			->willReturn($account);
 		$this->aliasesService->expects(self::once())
-			->method('findByAliasEmailAndUserId')
+			->method('findByAliasAndUserId')
 			->willReturn($alias);
 		$this->request->expects(self::once())
 			->method('getUploadedFile')
@@ -328,7 +328,7 @@ public function testAlias(): void {
 			->method('sendMessage')
 			->willReturn($this->message);
 
-		$expected = new DataResponse(null, Http::STATUS_OK);
+		$expected = new DataResponse('Success', Http::STATUS_OK);
 		$actual = $this->controller->send(
 			$accountId,
 			$aliasMail,
@@ -348,7 +348,7 @@ public function testNoAlias(): void {
 			->with($this->userId, $this->account->getId())
 			->willReturn($this->account);
 		$this->aliasesService->expects(self::once())
-			->method('findByAliasEmailAndUserId')
+			->method('findByAliasAndUserId')
 			->willThrowException(new DoesNotExistException(''));
 		$this->logger->expects(self::once())
 			->method('error');
@@ -384,7 +384,7 @@ public function testNoAccount(): void {
 		$this->logger->expects(self::once())
 			->method('error');
 		$this->aliasesService->expects(self::never())
-			->method('findByAliasEmailAndUserId');
+			->method('findByAliasAndUserId');
 		$this->request->expects(self::never())
 			->method('getUploadedFile');
 		$this->attachmentService->expects(self::never())

diff --git a/tests/Unit/Service/AntiSpamServiceTest.php b/tests/Unit/Service/AntiSpamServiceTest.php
@@ -50,6 +50,7 @@ class AntiSpamServiceTest extends TestCase {
 	private SmtpClientFactory|MockObject $smtpClientFactory;
 	private MockObject|ImapMessageMapper $imapMessageMapper;
 	private LoggerInterface|MockObject $logger;
+	private MailManager|MockObject $mailManager;
 
 	protected function setUp(): void {
 		parent::setUp();