upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-04-21 06:08:46 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 14

Merge pull request #47425 from nextcloud/fix/avoid-invalid-share-on-transfer-ownership

Browse source 
fix: promote re-shares when deleting the parent share
This commit is contained in:
Côme Chilliet 2024-11-04 10:09:58 +01:00 committed by GitHub
commit 901496276b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 285 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -150,16 +150,6 @@ class OwnershipTransferService {
$output
);
$destinationPath = $finalTarget . '/' . $path;
// restore the shares
$this->restoreShares(
$sourceUid,
$destinationUid,
$destinationPath,
$shares,
$output
);
// transfer the incoming shares
if ($transferIncomingShares === true) {
$sourceShares = $this->collectIncomingShares(
@ -184,6 +174,16 @@ class OwnershipTransferService {
$move
);
}
$destinationPath = $finalTarget . '/' . $path;
// restore the shares
$this->restoreShares(
$sourceUid,
$destinationUid,
$destinationPath,
$shares,
$output
);
}
private function sanitizeFolderName(string $name): string {

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

@ -18,6 +18,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;
@ -782,6 +783,7 @@ class Manager implements IManager {
* @param IShare $share
* @return IShare The share object
* @throws \InvalidArgumentException
* @throws GenericShareException
*/
public function updateShare(IShare $share) {
$expirationDateUpdated = false;
@ -1039,6 +1041,89 @@ class Manager implements IManager {
return $deletedShares;
}
/** Promote re-shares into direct shares so that target user keeps access */
protected function promoteReshares(IShare $share): void {
try {
$node = $share->getNode();
} catch (NotFoundException) {
/* Skip if node not found */
return;
}
$userIds = [];
if ($share->getShareType() === IShare::TYPE_USER) {
$userIds[] = $share->getSharedWith();
} elseif ($share->getShareType() === IShare::TYPE_GROUP) {
$group = $this->groupManager->get($share->getSharedWith());
$users = $group?->getUsers() ?? [];
foreach ($users as $user) {
/* Skip share owner */
if ($user->getUID() === $share->getShareOwner() || $user->getUID() === $share->getSharedBy()) {
continue;
}
$userIds[] = $user->getUID();
}
} else {
/* We only support user and group shares */
return;
}
$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);
if ($node instanceof Folder) {
/* We need to get all shares by this user to get subshares */
$shares = $provider->getSharesBy($userId, $shareType, null, false, -1, 0);
foreach ($shares as $share) {
try {
$path = $share->getNode()->getPath();
} catch (NotFoundException) {
/* Ignore share of non-existing node */
continue;
}
if ($node->getRelativePath($path) !== null) {
/* If relative path is not null it means the shared node is the same or in a subfolder */
$reshareRecords[] = $share;
}
}
} else {
$shares = $provider->getSharesBy($userId, $shareType, $node, false, -1, 0);
foreach ($shares as $child) {
$reshareRecords[] = $child;
}
}
}
}
foreach ($reshareRecords as $child) {
try {
/* Check if the share is still valid (means the resharer still has access to the file through another mean) */
$this->generalCreateChecks($child);
} catch (GenericShareException $e) {
/* The check is invalid, promote it to a direct share from the sharer of parent share */
$this->logger->debug('Promote reshare because of exception ' . $e->getMessage(), ['exception' => $e, 'fullId' => $child->getFullId()]);
try {
$child->setSharedBy($share->getSharedBy());
$this->updateShare($child);
} catch (GenericShareException|\InvalidArgumentException $e) {
$this->logger->warning('Failed to promote reshare because of exception ' . $e->getMessage(), ['exception' => $e, 'fullId' => $child->getFullId()]);
}
}
}
}
/**
* Delete a share
*
@ -1063,6 +1148,9 @@ class Manager implements IManager {
$provider->delete($share);
$this->dispatcher->dispatchTyped(new ShareDeletedEvent($share));
// Promote reshares of the deleted share
$this->promoteReshares($share);
}

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

@ -9,6 +9,7 @@ namespace Test\Share20;
use DateTimeZone;
use OC\Files\Mount\MoveableMount;
use OC\Files\Utils\PathHelper;
use OC\KnownUser\KnownUserService;
use OC\Share20\DefaultShareProvider;
use OC\Share20\Exception;
@ -46,6 +47,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;
@ -198,6 +200,14 @@ class ManagerTest extends \Test\TestCase {
]);
}
private function createFolderMock(string $folderPath): MockObject&Folder {
$folder = $this->createMock(Folder::class);
$folder->method('getPath')->willReturn($folderPath);
$folder->method('getRelativePath')->willReturnCallback(
fn (string $path): ?string => PathHelper::getRelativePath($folderPath, $path)
);
return $folder;
}
public function testDeleteNoShareId(): void {
$this->expectException(\InvalidArgumentException::class);
@ -227,7 +237,7 @@ class ManagerTest extends \Test\TestCase {
*/
public function testDelete($shareType, $sharedWith): void {
$manager = $this->createManagerMock()
->setMethods(['getShareById', 'deleteChildren'])
->setMethods(['getShareById', 'deleteChildren', 'promoteReshares'])
->getMock();
$manager->method('deleteChildren')->willReturn([]);
@ -245,6 +255,7 @@ class ManagerTest extends \Test\TestCase {
->setTarget('myTarget');
$manager->expects($this->once())->method('deleteChildren')->with($share);
$manager->expects($this->once())->method('promoteReshares')->with($share);
$this->defaultProvider
->expects($this->once())
@ -269,7 +280,7 @@ class ManagerTest extends \Test\TestCase {
public function testDeleteLazyShare(): void {
$manager = $this->createManagerMock()
->setMethods(['getShareById', 'deleteChildren'])
->setMethods(['getShareById', 'deleteChildren', 'promoteReshares'])
->getMock();
$manager->method('deleteChildren')->willReturn([]);
@ -288,6 +299,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('promoteReshares')->with($share);
$this->defaultProvider
->expects($this->once())
@ -312,7 +324,7 @@ class ManagerTest extends \Test\TestCase {
public function testDeleteNested(): void {
$manager = $this->createManagerMock()
->setMethods(['getShareById'])
->setMethods(['getShareById', 'promoteReshares'])
->getMock();
$path = $this->createMock(File::class);
@ -469,6 +481,178 @@ class ManagerTest extends \Test\TestCase {
$this->assertSame($shares, $result);
}
public function testPromoteReshareFile(): void {
$manager = $this->createManagerMock()
->setMethods(['updateShare', 'getSharesInFolder', 'generalCreateChecks'])
->getMock();
$file = $this->createMock(File::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($file);
$reShare = $this->createMock(IShare::class);
$reShare->method('getShareType')->willReturn(IShare::TYPE_USER);
$reShare->method('getSharedBy')->willReturn('userB');
$reShare->method('getSharedWith')->willReturn('userC');
$reShare->method('getNode')->willReturn($file);
$this->defaultProvider->method('getSharesBy')
->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($reShare, $file) {
$this->assertEquals($file, $node);
if ($shareType === IShare::TYPE_USER) {
return match($userId) {
'userB' => [$reShare],
};
} else {
return [];
}
});
$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
$manager->expects($this->exactly(1))->method('updateShare')->with($reShare);
self::invokePrivate($manager, 'promoteReshares', [$share]);
}
public function testPromoteReshare(): void {
$manager = $this->createManagerMock()
->setMethods(['updateShare', 'getSharesInFolder', 'generalCreateChecks'])
->getMock();
$folder = $this->createFolderMock('/path/to/folder');
$subFolder = $this->createFolderMock('/path/to/folder/sub');
$otherFolder = $this->createFolderMock('/path/to/otherfolder/');
$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('getSharedBy')->willReturn('userB');
$reShare->method('getSharedWith')->willReturn('userC');
$reShare->method('getNode')->willReturn($folder);
$reShareInSubFolder = $this->createMock(IShare::class);
$reShareInSubFolder->method('getShareType')->willReturn(IShare::TYPE_USER);
$reShareInSubFolder->method('getSharedBy')->willReturn('userB');
$reShareInSubFolder->method('getNode')->willReturn($subFolder);
$reShareInOtherFolder = $this->createMock(IShare::class);
$reShareInOtherFolder->method('getShareType')->willReturn(IShare::TYPE_USER);
$reShareInOtherFolder->method('getSharedBy')->willReturn('userB');
$reShareInOtherFolder->method('getNode')->willReturn($otherFolder);
$this->defaultProvider->method('getSharesBy')
->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($reShare, $reShareInSubFolder, $reShareInOtherFolder) {
if ($shareType === IShare::TYPE_USER) {
return match($userId) {
'userB' => [$reShare,$reShareInSubFolder,$reShareInOtherFolder],
};
} else {
return [];
}
});
$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
$manager->expects($this->exactly(2))->method('updateShare')->withConsecutive([$reShare], [$reShareInSubFolder]);
self::invokePrivate($manager, 'promoteReshares', [$share]);
}
public function testPromoteReshareWhenUserHasAnotherShare(): void {
$manager = $this->createManagerMock()
->setMethods(['updateShare', 'getSharesInFolder', 'getSharedWith', 'generalCreateChecks'])
->getMock();
$folder = $this->createFolderMock('/path/to/folder');
$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('generalCreateChecks')->willReturn(true);
/* No share is promoted because generalCreateChecks does not throw */
$manager->expects($this->never())->method('updateShare');
self::invokePrivate($manager, 'promoteReshares', [$share]);
}
public function testPromoteReshareOfUsersInGroupShare(): void {
$manager = $this->createManagerMock()
->setMethods(['updateShare', 'getSharesInFolder', 'getSharedWith', 'generalCreateChecks'])
->getMock();
$folder = $this->createFolderMock('/path/to/folder');
$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')
->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($reShare1, $reShare2) {
if ($shareType === IShare::TYPE_USER) {
return match($userId) {
'userB' => [$reShare1],
'userC' => [$reShare2],
};
} else {
return [];
}
});
$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
$manager->method('getSharedWith')->willReturn([]);
$manager->expects($this->exactly(2))->method('updateShare')->withConsecutive([$reShare1], [$reShare2]);
self::invokePrivate($manager, 'promoteReshares', [$share]);
}
public function testGetShareById(): void {
$share = $this->createMock(IShare::class);