Merge pull request #2425 from nextcloud/fix/2277/correct-reshare-notification-path

fix: correct reshare notification path

Author: miaulalala

lib/FilesHooks.php

@@ -1099,32 +1099,47 @@ protected function shareNotificationForOriginalOwners(string $sharedBy, string $
 		$mount = $fileSource->getMountPoint();
 		if ($mount instanceof SharedMount) {
 			$sourceShare = $mount->getShare();
-			try {
-				$sourceNode = $sourceShare->getNode();
-			} catch (NotFoundException) {
-				return;
-			}
+
+			$fileId = $fileSource->getId();
 
 			if ($sourceShare->getShareOwner() !== $sharedBy) {
+				$owner = $sourceShare->getShareOwner();
+				try {
+					$ownerNode = $this->rootFolder->getUserFolder($owner)->getFirstNodeById($fileId);
+				} catch (NotFoundException) {
+					return;
+				}
+				if ($ownerNode === null) {
+					return;
+				}
 				$this->reshareNotificationForSharer(
-					$sourceShare->getShareOwner(),
+					$owner,
 					$subject,
 					$shareWith,
-					$sourceNode->getId(),
-					$this->getUserRelativePath($sourceShare->getShareOwner(), $sourceNode->getPath()),
-					$sourceNode instanceof File,
+					$fileId,
+					$this->getUserRelativePath($owner, $ownerNode->getPath()),
+					$fileSource instanceof File,
 				);
 			}
 
-
 			if ($sourceShare->getSharedBy() && $sourceShare->getSharedBy() !== $sharedBy && $sourceShare->getShareOwner() !== $sourceShare->getSharedBy()) {
+				$sharer = $sourceShare->getSharedBy();
+				try {
+					$sharerNode = $this->rootFolder->getUserFolder($sharer)->getFirstNodeById($fileId);
+				} catch (NotFoundException) {
+					return;
+				}
+				if ($sharerNode === null) {
+					return;
+				}
+
 				$this->reshareNotificationForSharer(
-					$sourceShare->getSharedBy(),
+					$sharer,
 					$subject,
 					$shareWith,
-					$sourceNode->getId(),
-					$sourceShare->getTarget(),
-					$sourceNode instanceof File,
+					$fileId,
+					$this->getUserRelativePath($sharer, $sharerNode->getPath()),
+					$fileSource instanceof File,
 				);
 			}
 		}

tests/FilesHooksTest.php

@@ -828,19 +828,18 @@ public function testShare(): void {
 
 	public static function dataShareNotificationForOriginalOwners(): array {
 		return [
-			[false, false, 'owner', '', 0],
-			[true, false, 'owner', '', 1],
-			[true, true, 'owner', '', 1],
-			[true, true, 'owner', 'owner', 1],
-			[true, true, 'owner', 'sharee', 2],
-			[true, true, 'current', 'sharee', 1],
-			[true, true, 'owner', 'current', 1],
-			[true, true, 'current', 'current', 0],
+			[true, 'owner', '', 1],
+			[true, 'owner', 'owner', 1],
+			[true, 'owner', 'sharee', 2],
+			[true, 'current', 'sharee', 1],
+			[true, 'owner', 'current', 1],
+			[true, 'current', 'current', 0],
+			[false, 'owner', '', 0],
 		];
 	}
 
 	#[DataProvider('dataShareNotificationForOriginalOwners')]
-	public function testShareNotificationForOriginalOwners(bool $validMountPoint, bool $validSharedStorage, string $pathOwner, string $shareeUser, int $numReshareNotification): void {
+	public function testShareNotificationForOriginalOwners(bool $nodeFound, string $pathOwner, string $shareeUser, int $numReshareNotification): void {
 		$filesHooks = $this->getFilesHooks([
 			'reshareNotificationForSharer',
 		]);
@@ -861,20 +860,75 @@ public function testShareNotificationForOriginalOwners(bool $validMountPoint, bo
 
 		$filesHooks->expects($this->exactly($numReshareNotification))
 			->method('reshareNotificationForSharer')
-			->with($this->anything(), 'subject', 'with', 42, '/source-path', 'file');
-
-		if ($validMountPoint) {
-			$sourceNode = $this->getNodeMock(42, "/$pathOwner/files/source-path");
-			$sourceShare->method('getNode')
-				->willReturn($sourceNode);
+			->with($this->anything(), 'subject', 'with', 42, '/source-path', true);
+
+		// Mock rootFolder->getUserFolder()->getById() for each relevant user
+		if ($nodeFound) {
+			$this->rootFolder->method('getUserFolder')
+				->willReturnCallback(function (string $userId) {
+					$userFolder = $this->createMock(Folder::class);
+					$resolvedNode = $this->getNodeMock(42, "/$userId/files/source-path");
+					$userFolder->method('getById')
+						->with(42)
+						->willReturn([$resolvedNode]);
+					$userFolder->method('getFirstNodeById')
+						->with(42)
+						->willReturn($resolvedNode);
+					return $userFolder;
+				});
 		} else {
-			$sourceShare->method('getNode')
+			$this->rootFolder->method('getUserFolder')
 				->willThrowException(new \OCP\Files\NotFoundException());
 		}
 
 		self::invokePrivate($filesHooks, 'shareNotificationForOriginalOwners', ['current', 'subject', 'with', $node]);
 	}
 
+	/**
+	 * Test that resharing a subfolder notifies the owner about the subfolder,
+	 * not the parent folder (issue #2277).
+	 */
+	public function testShareNotificationForOriginalOwnersUsesActualFileNotShareRoot(): void {
+		$filesHooks = $this->getFilesHooks([
+			'reshareNotificationForSharer',
+		]);
+
+		// The actual subfolder being reshared (file ID 99, inside a shared parent)
+		$subfolder = $this->getNodeMock(99, '/userA/files/ParentFolder/Subfolder', false);
+		$mount = $this->createMock(SharedMount::class);
+		$subfolder->method('getMountPoint')
+			->willReturn($mount);
+
+		// The original share is for the parent folder (file ID 50)
+		$sourceShare = $this->createMock(IShare::class);
+		$sourceShare->method('getShareOwner')
+			->willReturn('admin');
+		$sourceShare->method('getSharedBy')
+			->willReturn('');
+		$mount->method('getShare')
+			->willReturn($sourceShare);
+
+		// The owner's view of the subfolder (resolved by file ID 99)
+		$ownerSubfolder = $this->getNodeMock(99, '/admin/files/ParentFolder/Subfolder', false);
+		$userFolder = $this->createMock(Folder::class);
+		$userFolder->method('getById')
+			->with(99)
+			->willReturn([$ownerSubfolder]);
+		$userFolder->method('getFirstNodeById')
+			->with(99)
+			->willReturn($ownerSubfolder);
+		$this->rootFolder->method('getUserFolder')
+			->with('admin')
+			->willReturn($userFolder);
+
+		// The notification must reference file ID 99 and /ParentFolder/Subfolder, NOT the parent
+		$filesHooks->expects($this->once())
+			->method('reshareNotificationForSharer')
+			->with('admin', 'reshared_user_by', 'userB', 99, '/ParentFolder/Subfolder', false);
+
+		self::invokePrivate($filesHooks, 'shareNotificationForOriginalOwners', ['userA', 'reshared_user_by', 'userB', $subfolder]);
+	}
+
 	public function testShareNotificationForSharer(): void {
 		$filesHooks = $this->getFilesHooks(['addNotificationsForUser']);