Skip to content

Commit b889eb2

Browse files
committed
fix: correct reshare notification path
Signed-off-by: Anna Larch <anna@nextcloud.com>
1 parent aae4c99 commit b889eb2

2 files changed

Lines changed: 99 additions & 30 deletions

File tree

lib/FilesHooks.php

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,32 +1099,47 @@ protected function shareNotificationForOriginalOwners(string $sharedBy, string $
10991099
$mount = $fileSource->getMountPoint();
11001100
if ($mount instanceof SharedMount) {
11011101
$sourceShare = $mount->getShare();
1102-
try {
1103-
$sourceNode = $sourceShare->getNode();
1104-
} catch (NotFoundException) {
1105-
return;
1106-
}
1102+
1103+
$fileId = $fileSource->getId();
11071104

11081105
if ($sourceShare->getShareOwner() !== $sharedBy) {
1106+
$owner = $sourceShare->getShareOwner();
1107+
try {
1108+
$ownerNode = $this->rootFolder->getUserFolder($owner)->getFirstNodeById($fileId);
1109+
} catch (NotFoundException) {
1110+
return;
1111+
}
1112+
if ($ownerNode === null) {
1113+
return;
1114+
}
11091115
$this->reshareNotificationForSharer(
1110-
$sourceShare->getShareOwner(),
1116+
$owner,
11111117
$subject,
11121118
$shareWith,
1113-
$sourceNode->getId(),
1114-
$this->getUserRelativePath($sourceShare->getShareOwner(), $sourceNode->getPath()),
1115-
$sourceNode instanceof File,
1119+
$fileId,
1120+
$this->getUserRelativePath($owner, $ownerNode->getPath()),
1121+
$fileSource instanceof File,
11161122
);
11171123
}
11181124

1119-
11201125
if ($sourceShare->getSharedBy() && $sourceShare->getSharedBy() !== $sharedBy && $sourceShare->getShareOwner() !== $sourceShare->getSharedBy()) {
1126+
$sharer = $sourceShare->getSharedBy();
1127+
try {
1128+
$sharerNode = $this->rootFolder->getUserFolder($sharer)->getFirstNodeById($fileId);
1129+
} catch (NotFoundException) {
1130+
return;
1131+
}
1132+
if ($sharerNode === null) {
1133+
return;
1134+
}
1135+
11211136
$this->reshareNotificationForSharer(
1122-
$sourceShare->getSharedBy(),
1137+
$sharer,
11231138
$subject,
11241139
$shareWith,
1125-
$sourceNode->getId(),
1126-
$sourceShare->getTarget(),
1127-
$sourceNode instanceof File,
1140+
$fileId,
1141+
$this->getUserRelativePath($sharer, $sharerNode->getPath()),
1142+
$fileSource instanceof File,
11281143
);
11291144
}
11301145
}

tests/FilesHooksTest.php

Lines changed: 70 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -828,19 +828,18 @@ public function testShare(): void {
828828

829829
public static function dataShareNotificationForOriginalOwners(): array {
830830
return [
831-
[false, false, 'owner', '', 0],
832-
[true, false, 'owner', '', 1],
833-
[true, true, 'owner', '', 1],
834-
[true, true, 'owner', 'owner', 1],
835-
[true, true, 'owner', 'sharee', 2],
836-
[true, true, 'current', 'sharee', 1],
837-
[true, true, 'owner', 'current', 1],
838-
[true, true, 'current', 'current', 0],
831+
[true, 'owner', '', 1],
832+
[true, 'owner', 'owner', 1],
833+
[true, 'owner', 'sharee', 2],
834+
[true, 'current', 'sharee', 1],
835+
[true, 'owner', 'current', 1],
836+
[true, 'current', 'current', 0],
837+
[false, 'owner', '', 0],
839838
];
840839
}
841840

842841
#[DataProvider('dataShareNotificationForOriginalOwners')]
843-
public function testShareNotificationForOriginalOwners(bool $validMountPoint, bool $validSharedStorage, string $pathOwner, string $shareeUser, int $numReshareNotification): void {
842+
public function testShareNotificationForOriginalOwners(bool $nodeFound, string $pathOwner, string $shareeUser, int $numReshareNotification): void {
844843
$filesHooks = $this->getFilesHooks([
845844
'reshareNotificationForSharer',
846845
]);
@@ -861,20 +860,75 @@ public function testShareNotificationForOriginalOwners(bool $validMountPoint, bo
861860

862861
$filesHooks->expects($this->exactly($numReshareNotification))
863862
->method('reshareNotificationForSharer')
864-
->with($this->anything(), 'subject', 'with', 42, '/source-path', 'file');
865-
866-
if ($validMountPoint) {
867-
$sourceNode = $this->getNodeMock(42, "/$pathOwner/files/source-path");
868-
$sourceShare->method('getNode')
869-
->willReturn($sourceNode);
863+
->with($this->anything(), 'subject', 'with', 42, '/source-path', true);
864+
865+
// Mock rootFolder->getUserFolder()->getById() for each relevant user
866+
if ($nodeFound) {
867+
$this->rootFolder->method('getUserFolder')
868+
->willReturnCallback(function (string $userId) {
869+
$userFolder = $this->createMock(Folder::class);
870+
$resolvedNode = $this->getNodeMock(42, "/$userId/files/source-path");
871+
$userFolder->method('getById')
872+
->with(42)
873+
->willReturn([$resolvedNode]);
874+
$userFolder->method('getFirstNodeById')
875+
->with(42)
876+
->willReturn($resolvedNode);
877+
return $userFolder;
878+
});
870879
} else {
871-
$sourceShare->method('getNode')
880+
$this->rootFolder->method('getUserFolder')
872881
->willThrowException(new \OCP\Files\NotFoundException());
873882
}
874883

875884
self::invokePrivate($filesHooks, 'shareNotificationForOriginalOwners', ['current', 'subject', 'with', $node]);
876885
}
877886

887+
/**
888+
* Test that resharing a subfolder notifies the owner about the subfolder,
889+
* not the parent folder (issue #2277).
890+
*/
891+
public function testShareNotificationForOriginalOwnersUsesActualFileNotShareRoot(): void {
892+
$filesHooks = $this->getFilesHooks([
893+
'reshareNotificationForSharer',
894+
]);
895+
896+
// The actual subfolder being reshared (file ID 99, inside a shared parent)
897+
$subfolder = $this->getNodeMock(99, '/userA/files/ParentFolder/Subfolder', false);
898+
$mount = $this->createMock(SharedMount::class);
899+
$subfolder->method('getMountPoint')
900+
->willReturn($mount);
901+
902+
// The original share is for the parent folder (file ID 50)
903+
$sourceShare = $this->createMock(IShare::class);
904+
$sourceShare->method('getShareOwner')
905+
->willReturn('admin');
906+
$sourceShare->method('getSharedBy')
907+
->willReturn('');
908+
$mount->method('getShare')
909+
->willReturn($sourceShare);
910+
911+
// The owner's view of the subfolder (resolved by file ID 99)
912+
$ownerSubfolder = $this->getNodeMock(99, '/admin/files/ParentFolder/Subfolder', false);
913+
$userFolder = $this->createMock(Folder::class);
914+
$userFolder->method('getById')
915+
->with(99)
916+
->willReturn([$ownerSubfolder]);
917+
$userFolder->method('getFirstNodeById')
918+
->with(99)
919+
->willReturn($ownerSubfolder);
920+
$this->rootFolder->method('getUserFolder')
921+
->with('admin')
922+
->willReturn($userFolder);
923+
924+
// The notification must reference file ID 99 and /ParentFolder/Subfolder, NOT the parent
925+
$filesHooks->expects($this->once())
926+
->method('reshareNotificationForSharer')
927+
->with('admin', 'reshared_user_by', 'userB', 99, '/ParentFolder/Subfolder', false);
928+
929+
self::invokePrivate($filesHooks, 'shareNotificationForOriginalOwners', ['userA', 'reshared_user_by', 'userB', $subfolder]);
930+
}
931+
878932
public function testShareNotificationForSharer(): void {
879933
$filesHooks = $this->getFilesHooks(['addNotificationsForUser']);
880934

0 commit comments

Comments
 (0)