upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-05-17 02:49:55 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 11

fix: delete re-shares when deleting the parent share

Browse source 
Note: Removed part about fix command from original PR

Signed-off-by: Luka Trovic <luka@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
(cherry picked from commit 42181c2f49)
This commit is contained in:
Luka Trovic 2024-08-07 11:13:35 +02:00 committed by backportbot[bot]
parent 8de467c60e
commit dfbaa103f9
4 changed files with 190 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
apps/files/lib/Service/OwnershipTransferService.php
View file

@ -488,6 +488,9 @@ class OwnershipTransferService {
}
} catch (\OCP\Files\NotFoundException $e) {
$output->writeln('<error>Share with id ' . $share->getId() . ' points at deleted file, skipping</error>');
} catch (\OCP\Share\Exceptions\GenericShareException $e) {
$output->writeln('<error>Share with id ' . $share->getId() . ' is broken, deleting</error>');
$this->shareManager->deleteShare($share);
} catch (\Throwable $e) {
$output->writeln('<error>Could not restore share with id ' . $share->getId() . ':' . $e->getMessage() . ' : ' . $e->getTraceAsString() . '</error>');
}

3
apps/files_sharing/tests/EtagPropagationTest.php
View file

@ -299,7 +299,8 @@ class EtagPropagationTest extends PropagationTestCase {
self::TEST_FILES_SHARING_API_USER2,
]);
$this->assertAllUnchanged();
$this->assertEtagsNotChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER3]);
$this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER4]);
}
public function testOwnerUnsharesFlatReshares() {

69
lib/private/Share20/Manager.php
View file

@ -52,6 +52,7 @@ use OCP\Files\Folder;
use OCP\Files\IRootFolder;
use OCP\Files\Mount\IMountManager;
use OCP\Files\Node;
use OCP\Files\NotFoundException;
use OCP\HintException;
use OCP\IConfig;
use OCP\IDateTimeZone;
@ -1163,6 +1164,71 @@ class Manager implements IManager {
return $deletedShares;
}
public function deleteReshare(IShare $share) {
// Skip if node not found
try {
$node = $share->getNode();
} catch (NotFoundException) {
return;
}
$userIds = [];
if ($share->getShareType() === IShare::TYPE_USER) {
$userIds[] = $share->getSharedWith();
}
if ($share->getShareType() === IShare::TYPE_GROUP) {
$group = $this->groupManager->get($share->getSharedWith());
$users = $group->getUsers();
foreach ($users as $user) {
// Skip if share owner is member of shared group
if ($user->getUID() === $share->getShareOwner()) {
continue;
}
$userIds[] = $user->getUID();
}
}
$reshareRecords = [];
$shareTypes = [
IShare::TYPE_GROUP,
IShare::TYPE_USER,
IShare::TYPE_LINK,
IShare::TYPE_REMOTE,
IShare::TYPE_EMAIL
];
foreach ($userIds as $userId) {
foreach ($shareTypes as $shareType) {
$provider = $this->factory->getProviderForType($shareType);
$shares = $provider->getSharesBy($userId, $shareType, $node, false, -1, 0);
foreach ($shares as $child) {
$reshareRecords[] = $child;
}
}
if ($share->getNodeType() === 'folder') {
$sharesInFolder = $this->getSharesInFolder($userId, $node, true);
foreach ($sharesInFolder as $shares) {
foreach ($shares as $child) {
$reshareRecords[] = $child;
}
}
}
}
foreach ($reshareRecords as $child) {
try {
$this->generalCreateChecks($child);
} catch (GenericShareException $e) {
$this->deleteShare($child);
}
}
}
/**
* Delete a share
*
@ -1186,6 +1252,9 @@ class Manager implements IManager {
$provider = $this->factory->getProviderForType($share->getShareType());
$provider->delete($share);
// Delete shares that shared by the "share with user/group"
$this->deleteReshare($share);
$this->dispatcher->dispatchTyped(new ShareDeletedEvent($share));
}

120
tests/lib/Share20/ManagerTest.php
View file

@ -60,6 +60,7 @@ use OCP\Share\Events\ShareCreatedEvent;
use OCP\Share\Events\ShareDeletedEvent;
use OCP\Share\Events\ShareDeletedFromSelfEvent;
use OCP\Share\Exceptions\AlreadySharedException;
use OCP\Share\Exceptions\GenericShareException;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IManager;
use OCP\Share\IProviderFactory;
@ -241,7 +242,7 @@ class ManagerTest extends \Test\TestCase {
*/
public function testDelete($shareType, $sharedWith) {
$manager = $this->createManagerMock()
->setMethods(['getShareById', 'deleteChildren'])
->setMethods(['getShareById', 'deleteChildren', 'deleteReshare'])
->getMock();
$manager->method('deleteChildren')->willReturn([]);
@ -259,6 +260,7 @@ class ManagerTest extends \Test\TestCase {
->setTarget('myTarget');
$manager->expects($this->once())->method('deleteChildren')->with($share);
$manager->expects($this->once())->method('deleteReshare')->with($share);
$this->defaultProvider
->expects($this->once())
@ -283,7 +285,7 @@ class ManagerTest extends \Test\TestCase {
public function testDeleteLazyShare() {
$manager = $this->createManagerMock()
->setMethods(['getShareById', 'deleteChildren'])
->setMethods(['getShareById', 'deleteChildren', 'deleteReshare'])
->getMock();
$manager->method('deleteChildren')->willReturn([]);
@ -302,6 +304,7 @@ class ManagerTest extends \Test\TestCase {
$this->rootFolder->expects($this->never())->method($this->anything());
$manager->expects($this->once())->method('deleteChildren')->with($share);
$manager->expects($this->once())->method('deleteReshare')->with($share);
$this->defaultProvider
->expects($this->once())
@ -326,7 +329,7 @@ class ManagerTest extends \Test\TestCase {
public function testDeleteNested() {
$manager = $this->createManagerMock()
->setMethods(['getShareById'])
->setMethods(['getShareById', 'deleteReshare'])
->getMock();
$path = $this->createMock(File::class);
@ -483,7 +486,116 @@ class ManagerTest extends \Test\TestCase {
$this->assertSame($shares, $result);
}
public function testGetShareById() {
public function testDeleteReshareWhenUserHasOneShare(): void {
$manager = $this->createManagerMock()
->setMethods(['deleteShare', 'getSharesInFolder', 'generalCreateChecks'])
->getMock();
$folder = $this->createMock(Folder::class);
$share = $this->createMock(IShare::class);
$share->method('getShareType')->willReturn(IShare::TYPE_USER);
$share->method('getNodeType')->willReturn('folder');
$share->method('getSharedWith')->willReturn('UserB');
$share->method('getNode')->willReturn($folder);
$reShare = $this->createMock(IShare::class);
$reShare->method('getSharedBy')->willReturn('UserB');
$reShare->method('getSharedWith')->willReturn('UserC');
$reShare->method('getNode')->willReturn($folder);
$reShareInSubFolder = $this->createMock(IShare::class);
$reShareInSubFolder->method('getSharedBy')->willReturn('UserB');
$manager->method('getSharesInFolder')->willReturn([$reShareInSubFolder]);
$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
$this->defaultProvider->method('getSharesBy')
->willReturn([$reShare]);
$manager->expects($this->atLeast(2))->method('deleteShare')->withConsecutive([$reShare], [$reShareInSubFolder]);
$manager->deleteReshare($share);
}
public function testDeleteReshareWhenUserHasAnotherShare(): void {
$manager = $this->createManagerMock()
->setMethods(['deleteShare', 'getSharesInFolder', 'getSharedWith', 'generalCreateChecks'])
->getMock();
$folder = $this->createMock(Folder::class);
$share = $this->createMock(IShare::class);
$share->method('getShareType')->willReturn(IShare::TYPE_USER);
$share->method('getNodeType')->willReturn('folder');
$share->method('getSharedWith')->willReturn('UserB');
$share->method('getNode')->willReturn($folder);
$reShare = $this->createMock(IShare::class);
$reShare->method('getShareType')->willReturn(IShare::TYPE_USER);
$reShare->method('getNodeType')->willReturn('folder');
$reShare->method('getSharedBy')->willReturn('UserB');
$reShare->method('getNode')->willReturn($folder);
$this->defaultProvider->method('getSharesBy')->willReturn([$reShare]);
$manager->method('getSharesInFolder')->willReturn([]);
$manager->method('generalCreateChecks')->willReturn(true);
$manager->expects($this->never())->method('deleteShare');
$manager->deleteReshare($share);
}
public function testDeleteReshareOfUsersInGroupShare(): void {
$manager = $this->createManagerMock()
->setMethods(['deleteShare', 'getSharesInFolder', 'getSharedWith', 'generalCreateChecks'])
->getMock();
$folder = $this->createMock(Folder::class);
$userA = $this->createMock(IUser::class);
$userA->method('getUID')->willReturn('userA');
$share = $this->createMock(IShare::class);
$share->method('getShareType')->willReturn(IShare::TYPE_GROUP);
$share->method('getNodeType')->willReturn('folder');
$share->method('getSharedWith')->willReturn('Group');
$share->method('getNode')->willReturn($folder);
$share->method('getShareOwner')->willReturn($userA);
$reShare1 = $this->createMock(IShare::class);
$reShare1->method('getShareType')->willReturn(IShare::TYPE_USER);
$reShare1->method('getNodeType')->willReturn('folder');
$reShare1->method('getSharedBy')->willReturn('UserB');
$reShare1->method('getNode')->willReturn($folder);
$reShare2 = $this->createMock(IShare::class);
$reShare2->method('getShareType')->willReturn(IShare::TYPE_USER);
$reShare2->method('getNodeType')->willReturn('folder');
$reShare2->method('getSharedBy')->willReturn('UserC');
$reShare2->method('getNode')->willReturn($folder);
$userB = $this->createMock(IUser::class);
$userB->method('getUID')->willReturn('userB');
$userC = $this->createMock(IUser::class);
$userC->method('getUID')->willReturn('userC');
$group = $this->createMock(IGroup::class);
$group->method('getUsers')->willReturn([$userB, $userC]);
$this->groupManager->method('get')->with('Group')->willReturn($group);
$this->defaultProvider->method('getSharesBy')
->willReturn([]);
$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
$manager->method('getSharedWith')->willReturn([]);
$manager->expects($this->exactly(2))->method('getSharesInFolder')->willReturnOnConsecutiveCalls([[$reShare1]], [[$reShare2]]);
$manager->expects($this->exactly(2))->method('deleteShare')->withConsecutive([$reShare1], [$reShare2]);
$manager->deleteReshare($share);
}
public function testGetShareById(): void {
$share = $this->createMock(IShare::class);
$this->defaultProvider