Merge pull request #37555 from owncloud/fix/mail-resource-path

use relative path in shared_by_email activity

apps/files_sharing/lib/Controller/NotificationController.php

@@ -25,6 +25,7 @@
 use OC\Share\MailNotifications;
 use OCP\AppFramework\OCSController;
 use OCP\Files\IRootFolder;
+use OCP\Files\NotFoundException;
 use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\IRequest;
@@ -88,7 +89,7 @@ public function notifyPublicLinkRecipientsByEmail($link, $recipients, $personalN
 					$link,
 					$personalNote
 				);
-			} catch (GenericShareException $e) {
+			} catch (GenericShareException | NotFoundException $e) {
 				$code = $e->getCode() === 0 ? 403 : $e->getCode();
 				return new Result(null, $code, $e->getHint());
 			}

changelog/unreleased/37555

@@ -0,0 +1,6 @@
+Bugfix: Use relative path in shared_with_email activity
+
+"shared_with_email" activity email was including the complete path of the shared node.
+This path has changed with the relative path of the sender user folder.
+
+https://github.com/owncloud/core/pull/37555

lib/private/Share/MailNotifications.php

@@ -30,7 +30,9 @@
 
 namespace OC\Share;
 
+use OCP\Files\IRootFolder;
 use OCP\Files\Node;
+use OCP\Files\NotFoundException;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\IURLGenerator;
@@ -71,6 +73,8 @@ class MailNotifications {
 	private $eventDispatcher;
 	/** @var \OCP\Activity\IManager */
 	private $activityManager;
+	/** @var IRootFolder */
+	private $rootFolder;
 
 	/**
 	 * @param IManager $shareManager
@@ -92,7 +96,8 @@ public function __construct(
 		Defaults $defaults,
 		IURLGenerator $urlGenerator,
 		EventDispatcher $eventDispatcher,
-		\OCP\Activity\IManager $activityManager
+		\OCP\Activity\IManager $activityManager,
+		IRootFolder $rootFolder
 	) {
 		$this->shareManager = $shareManager;
 		$this->l = $l10n;
@@ -103,6 +108,7 @@ public function __construct(
 		$this->urlGenerator = $urlGenerator;
 		$this->eventDispatcher = $eventDispatcher;
 		$this->activityManager = $activityManager;
+		$this->rootFolder = $rootFolder;
 	}
 
 	/**
@@ -213,6 +219,7 @@ public function sendInternalShareMail($sender, $node, $shareType, $recipientList
 	 * @param string $link the share link
 	 * @param string $personalNote sender note
 	 * @throws GenericShareException
+	 * @throws NotFoundException
 	 * @return string[] $result of failed recipients
 	 */
 	public function sendLinkShareMail($sender, $recipients, $link, $personalNote = null) {
@@ -272,7 +279,11 @@ public function sendLinkShareMail($sender, $recipients, $link, $personalNote = n
 		$failedRecipients =  $this->sendLinkShareMailFromBody($recipients, $subject, $htmlBody, $textBody, $from, $replyTo);
 		if (empty($failedRecipients)) {
 			$event = $this->activityManager->generateEvent();
-			$path = $shareNode->getPath();
+			// it can be a re-share, get actual share file node
+			$userFolder = $this->rootFolder->getUserFolder($sender->getUID());
+			$shareFileNodes = $userFolder->getById($shareNode->getId(), true);
+			$shareFileNode = $shareFileNodes[0] ?? null;
+			$path = $userFolder->getRelativePath($shareFileNode->getPath());
 			$event->setApp(\OCA\Files_Sharing\Activity::FILES_SHARING_APP)
 				->setType(\OCA\Files_Sharing\Activity::TYPE_SHARED)
 				->setAuthor($currentUser)

tests/lib/Share/MailNotificationsTest.php

@@ -24,6 +24,8 @@
 use OC\Mail\Message;
 use OC\Share\MailNotifications;
 use OCP\Defaults;
+use OCP\Files\Folder;
+use OCP\Files\IRootFolder;
 use OCP\Files\Node;
 use OCP\IConfig;
 use OCP\IL10N;
@@ -66,6 +68,8 @@ class MailNotificationsTest extends TestCase {
 	private $eventDispatcher;
 	/** @var \OCP\Activity\IManager | MockObject */
 	private $activityManager;
+	/** @var IRootFolder | MockObject */
+	private $rootFolder;
 	/** @var MailNotifications */
 	private $mailNotifications;
 
@@ -80,6 +84,7 @@ public function setUp(): void {
 		$this->defaults = $this->createMock(Defaults::class);
 		$this->user = $this->createMock(IUser::class);
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
+		$this->rootFolder = $this->createMock(IRootFolder::class);
 		$this->activityManager = \OC::$server->getActivityManager();
 		$this->eventDispatcher = new EventDispatcher();
 
@@ -113,7 +118,8 @@ public function setUp(): void {
 			$this->defaults,
 			$this->urlGenerator,
 			$this->eventDispatcher,
-			$this->activityManager
+			$this->activityManager,
+			$this->rootFolder
 		);
 	}
 
@@ -128,6 +134,12 @@ private function createMockShare($filename, $expiration=null) {
 		$node->method('getPath')->willReturn('/testuser/' . $filename);
 		$node->method('getName')->willReturn($filename);
 
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')->willReturn([$node]);
+		$userFolder->method('getRelativePath')->willReturn($filename);
+		$this->rootFolder->method('getUserFolder')
+			->with('testuser')->willReturn($userFolder);
+
 		$share = $this->createMock(IShare::class);
 		$share->method('getExpirationDate')->willReturn($expirationDate);
 		$share->method('getNodeId')->willReturn(1);
@@ -627,8 +639,6 @@ public function testSendInternalShareWithRecipientLanguageCode($recipientLanguag
 		$this->assertSame([], $result);
 	}
 
-	/**
-	 */
 	public function testSendLinkShareMailIfObeysConfig() {
 		$this->expectException(\OCP\Share\Exceptions\GenericShareException::class);
 
@@ -643,11 +653,9 @@ public function testSendLinkShareMailIfObeysConfig() {
 		);
 	}
 
-	/**
-	 */
 	public function testSendInternalShareMailIfObeysConfig() {
 		$this->expectException(\OCP\Share\Exceptions\GenericShareException::class);
-
+		$recipient = $this->createMock(IUser::class);
 		$this->config
 			->method('getAppValue')
 			->with('core', 'shareapi_allow_mail_notification', 'no')
@@ -656,7 +664,7 @@ public function testSendInternalShareMailIfObeysConfig() {
 			$this->user,
 			'3',
 			'file',
-			['test@user']
+			[$recipient]
 		);
 	}
 }