Merge pull request #36161 from owncloud/obey-mail-config

obey to config in share mail notifications APIs
owncloud · Sep 11, 2019 · 0dafa3f · 0dafa3f
2 parents 4941f02 + 8c962f8
commit 0dafa3f
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 13 deletions.
diff --git a/apps/files_sharing/lib/Controller/NotificationController.php b/apps/files_sharing/lib/Controller/NotificationController.php
@@ -32,6 +32,7 @@
 use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\Share;
+use OCP\Share\Exceptions\GenericShareException;
 
 class NotificationController extends OCSController {
 	/** @var MailNotifications */
@@ -80,12 +81,17 @@ public function notifyPublicLinkRecipientsByEmail($link, $recipients, $personalN
 		$sender = $this->userSession->getUser();
 		$result = $recipients;
 		if ($sender) {
-			$result = $this->mailNotifications->sendLinkShareMail(
-				$sender,
-				$recipients,
-				$link,
-				$personalNote
-			);
+			try {
+				$result = $this->mailNotifications->sendLinkShareMail(
+					$sender,
+					$recipients,
+					$link,
+					$personalNote
+				);
+			} catch (GenericShareException $e) {
+				$code = $e->getCode() === 0 ? 403 : $e->getCode();
+				return new Result(null, $code, $e->getHint());
+			}
 		}
 		if (!empty($result)) {
 			$message = $this->l->t("Couldn't send mail to following recipient(s): %s ",
@@ -127,7 +133,12 @@ public function notifyRecipients($itemSource, $itemType, $shareType, $recipient)
 		$userFolder = $this->rootFolder->getUserFolder($sender->getUID());
 		$nodes = $userFolder->getById($itemSource, true);
 		$node = $nodes[0] ?? null;
-		$result = $this->mailNotifications->sendInternalShareMail($sender, $node, $shareType, $recipientList);
+		try {
+			$result = $this->mailNotifications->sendInternalShareMail($sender, $node, $shareType, $recipientList);
+		} catch (GenericShareException $e) {
+			$code = $e->getCode() === 0 ? 403 : $e->getCode();
+			return new Result(null, $code, $e->getHint());
+		}
 
 		// if we were able to send to at least one recipient, mark as sent
 		// allowing the user to resend would spam users who already got a notification

diff --git a/lib/private/Share/MailNotifications.php b/lib/private/Share/MailNotifications.php
@@ -38,6 +38,7 @@
 use OCP\Mail\IMailer;
 use OCP\ILogger;
 use OCP\Defaults;
+use OCP\Share\Exceptions\GenericShareException;
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager;
 use OCP\Share\IShare;
@@ -111,10 +112,14 @@ public function __construct(
 	 * @param Node shared node
 	 * @param string $shareType share type
 	 * @param IUser[] $recipientList list of recipients
-	 *
+	 * @throws GenericShareException
 	 * @return array list of user to whom the mail send operation failed
 	 */
 	public function sendInternalShareMail($sender, $node, $shareType, $recipientList) {
+		if ($this->config->getAppValue('core', 'shareapi_allow_mail_notification', 'no') !== 'yes') {
+			$message_t = $this->l->t("Internal mail notification for shared files is not allowed");
+			throw new GenericShareException($message_t, $message_t, 403);
+		}
 		$noMail = [];
 
 		foreach ($recipientList as $recipient) {
@@ -205,10 +210,14 @@ public function sendInternalShareMail($sender, $node, $shareType, $recipientList
 	 * @param string[] $recipients recipient email addresses
 	 * @param string $link the share link
 	 * @param string $personalNote sender note
-	 *
+	 * @throws GenericShareException
 	 * @return string[] $result of failed recipients
 	 */
 	public function sendLinkShareMail($sender, $recipients, $link, $personalNote = null) {
+		if ($this->config->getAppValue('core', 'shareapi_allow_public_notification', 'no') !== 'yes') {
+			$message_t = $this->l->t("Public link mail notification is not allowed");
+			throw new GenericShareException($message_t, $message_t, 403);
+		}
 		$recipientsAsString = \implode(', ', $recipients);
 		$currentUser = $sender->getUID();
 		$notificationLang = $this->config->getAppValue(

diff --git a/tests/lib/Share/MailNotificationsTest.php b/tests/lib/Share/MailNotificationsTest.php
@@ -137,6 +137,13 @@ private function createMockShare($filename, $expiration=null) {
 	}
 
 	public function testSendLinkShareMailWithoutReplyTo() {
+		$this->config
+			->method('getAppValue')
+			->withConsecutive(
+				['core', 'shareapi_allow_public_notification', 'no'],
+				['core', 'shareapi_public_notification_lang', null]
+			)
+			->willReturnOnConsecutiveCalls('yes', null);
 		$message = $this->createMock(Message::class);
 		$message
 			->expects($this->once())
@@ -181,6 +188,13 @@ public function testSendLinkShareMailWithoutReplyTo() {
 	}
 
 	public function testSendLinkShareMailPersonalNote() {
+		$this->config
+			->method('getAppValue')
+			->withConsecutive(
+				['core', 'shareapi_allow_public_notification', 'no'],
+				['core', 'shareapi_public_notification_lang', null]
+			)
+			->willReturnOnConsecutiveCalls('yes', null);
 		$message = $this->createMock(Message::class);
 		$message
 			->expects($this->once())
@@ -242,6 +256,13 @@ public function testSendLinkShareMailPersonalNote() {
 	}
 
 	public function testSendLinkShareMailWithReplyTo() {
+		$this->config
+			->method('getAppValue')
+			->withConsecutive(
+				['core', 'shareapi_allow_public_notification', 'no'],
+				['core', 'shareapi_public_notification_lang', null]
+			)
+			->willReturnOnConsecutiveCalls('yes', null);
 		$message = $this->createMock(Message::class);
 		$message
 			->expects($this->exactly(2))
@@ -301,6 +322,13 @@ public function dataSendLinkShareMailException() {
 	 * @param array $to
 	 */
 	public function testSendLinkShareMailException($to) {
+		$this->config
+			->method('getAppValue')
+			->withConsecutive(
+				['core', 'shareapi_allow_public_notification', 'no'],
+				['core', 'shareapi_public_notification_lang', null]
+			)
+			->willReturnOnConsecutiveCalls('yes', null);
 		$this->setupMailerMock('TestUser shared »MyFile« with you', $to);
 		$this->shareManager
 			->method('getShareByToken')
@@ -317,6 +345,10 @@ public function testSendLinkShareMailException($to) {
 	}
 
 	public function testSendInternalShareMail() {
+		$this->config
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_mail_notification', 'no')
+			->willReturn('yes');
 		$this->setupMailerMock('TestUser shared »<welcome>.txt« with you', false);
 
 		$shareMock = $this->getShareMock(
@@ -354,6 +386,10 @@ public function testSendInternalShareMail() {
 	}
 
 	public function testSendInternalShareMailException() {
+		$this->config
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_mail_notification', 'no')
+			->willReturn('yes');
 		$this->setupMailerMock('TestUser shared »<welcome>.txt« with you', false);
 
 		$share = $this->getShareMock(
@@ -407,6 +443,10 @@ public function emptinessProvider() {
 	 * @dataProvider emptinessProvider
 	 */
 	public function testSendInternalShareMailNoMail($emptiness) {
+		$this->config
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_mail_notification', 'no')
+			->willReturn('yes');
 		$recipient = $this->createMock(IUser::class);
 		$recipient
 				->expects($this->once())
@@ -438,11 +478,13 @@ public function testSendInternalShareMailNoMail($emptiness) {
 	}
 
 	public function testPublicLinkNotificationIsTranslated() {
-		$this->config->expects($this->once())
+		$this->config
 			->method('getAppValue')
-			->with('core', 'shareapi_public_notification_lang', null)
-			->willReturn('ru');
-
+			->withConsecutive(
+				['core', 'shareapi_allow_public_notification', 'no'],
+				['core', 'shareapi_public_notification_lang', null]
+			)
+			->willReturnOnConsecutiveCalls('yes', 'ru');
 		$message = $this->createMock(Message::class);
 		$message
 			->expects($this->once())
@@ -533,6 +575,10 @@ public function providesLanguages() {
 	 * @param string $senderLanguage
 	 */
 	public function testSendInternalShareWithRecipientLanguageCode($recipientLanguage, $senderLanguage) {
+		$this->config
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_mail_notification', 'no')
+			->willReturn('yes');
 		$this->setupMailerMock('TestUser shared »<welcome>.txt« with you', false);
 		$shareMock = $this->getShareMock(
 			['file_target' => '/<welcome>.txt', 'item_source' => 123, 'expiration' => '2017-01-01T15:03:01.012345Z']
@@ -572,4 +618,35 @@ public function testSendInternalShareWithRecipientLanguageCode($recipientLanguag
 		);
 		$this->assertSame([], $result);
 	}
+
+	/**
+	 * @expectedException \OCP\Share\Exceptions\GenericShareException
+	 */
+	public function testSendLinkShareMailIfObeysConfig() {
+		$this->config
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_public_notification', 'no')
+			->willReturn('no');
+		$this->mailNotifications->sendLinkShareMail(
+			$this->user,
+			['test@user'],
+			''
+		);
+	}
+
+	/**
+	 * @expectedException \OCP\Share\Exceptions\GenericShareException
+	 */
+	public function testSendInternalShareMailIfObeysConfig() {
+		$this->config
+			->method('getAppValue')
+			->with('core', 'shareapi_allow_mail_notification', 'no')
+			->willReturn('no');
+		$this->mailNotifications->sendInternalShareMail(
+			$this->user,
+			'3',
+			'file',
+			['test@user']
+		);
+	}
 }