diff options
-rw-r--r-- | lib/private/Share20/Manager.php | 57 | ||||
-rw-r--r-- | tests/lib/Share20/ManagerTest.php | 23 |
2 files changed, 25 insertions, 55 deletions
diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 53dbf65ccc7..27478a4f708 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -45,7 +45,6 @@ use OC\Files\Mount\MoveableMount; use OC\KnownUser\KnownUserService; use OC\Share20\Exception\ProviderException; use OCA\Files_Sharing\AppInfo\Application; -use OCA\Files_Sharing\ISharedStorage; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\File; use OCP\Files\Folder; @@ -294,55 +293,15 @@ class Manager implements IManager { throw new \InvalidArgumentException('A share requires permissions'); } - $isFederatedShare = $share->getNode()->getStorage()->instanceOfStorage('\OCA\Files_Sharing\External\Storage'); $permissions = 0; - - $isReshare = $share->getNode()->getOwner() && $share->getNode()->getOwner()->getUID() !== $share->getSharedBy(); - if (!$isReshare && $isUpdate) { - // in case of update on owner-less filesystem, we use share owner to improve reshare detection - $isReshare = $share->getShareOwner() !== $share->getSharedBy(); - } - - if (!$isFederatedShare && $isReshare) { - $userMounts = array_filter($userFolder->getById($share->getNode()->getId()), function ($mount) { - // We need to filter since there might be other mountpoints that contain the file - // e.g. if the user has access to the same external storage that the file is originating from - return $mount->getStorage()->instanceOfStorage(ISharedStorage::class); - }); - $userMount = array_shift($userMounts); - if ($userMount === null) { - throw new GenericShareException('Could not get proper share mount for ' . $share->getNode()->getId() . '. Failing since else the next calls are called with null'); - } - $mount = $userMount->getMountPoint(); - // When it's a reshare use the parent share permissions as maximum - $userMountPointId = $mount->getStorageRootId(); - $userMountPoint = $userFolder->getFirstNodeById($userMountPointId); - - if ($userMountPoint === null) { - throw new GenericShareException('Could not get proper user mount for ' . $userMountPointId . '. Failing since else the next calls are called with null'); - } - - /* Check if this is an incoming share */ - $incomingShares = $this->getSharedWith($share->getSharedBy(), IShare::TYPE_USER, $userMountPoint, -1, 0); - $incomingShares = array_merge($incomingShares, $this->getSharedWith($share->getSharedBy(), IShare::TYPE_GROUP, $userMountPoint, -1, 0)); - $incomingShares = array_merge($incomingShares, $this->getSharedWith($share->getSharedBy(), IShare::TYPE_CIRCLE, $userMountPoint, -1, 0)); - $incomingShares = array_merge($incomingShares, $this->getSharedWith($share->getSharedBy(), IShare::TYPE_ROOM, $userMountPoint, -1, 0)); - - /** @var IShare[] $incomingShares */ - if (!empty($incomingShares)) { - foreach ($incomingShares as $incomingShare) { - $permissions |= $incomingShare->getPermissions(); - } - } - } else { - /* - * Quick fix for #23536 - * Non moveable mount points do not have update and delete permissions - * while we 'most likely' do have that on the storage. - */ - $permissions = $share->getNode()->getPermissions(); - if (!($share->getNode()->getMountPoint() instanceof MoveableMount)) { - $permissions |= \OCP\Constants::PERMISSION_DELETE | \OCP\Constants::PERMISSION_UPDATE; + $nodesForUser = $userFolder->getById($share->getNodeId()); + foreach ($nodesForUser as $node) { + if ($node->getInternalPath() === '' && !$node->getMountPoint() instanceof MoveableMount) { + // for the root of non-movable mount, the permissions we see if limited by the mount itself, + // so we instead use the "raw" permissions from the storage + $permissions |= $node->getStorage()->getPermissions(''); + } else { + $permissions |= $node->getPermissions(); } } diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index da173dc2e1f..daa8c1a887b 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -615,7 +615,7 @@ class ManagerTest extends \Test\TestCase { self::invokePrivate($this->manager, 'verifyPassword', ['password']); } - public function createShare($id, $type, $path, $sharedWith, $sharedBy, $shareOwner, + public function createShare($id, $type, $node, $sharedWith, $sharedBy, $shareOwner, $permissions, $expireDate = null, $password = null, $attributes = null) { $share = $this->createMock(IShare::class); @@ -623,7 +623,10 @@ class ManagerTest extends \Test\TestCase { $share->method('getSharedWith')->willReturn($sharedWith); $share->method('getSharedBy')->willReturn($sharedBy); $share->method('getShareOwner')->willReturn($shareOwner); - $share->method('getNode')->willReturn($path); + $share->method('getNode')->willReturn($node); + if ($node && $node->getId()) { + $share->method('getNodeId')->willReturn($node->getId()); + } $share->method('getPermissions')->willReturn($permissions); $share->method('getAttributes')->willReturn($attributes); $share->method('getExpirationDate')->willReturn($expireDate); @@ -648,8 +651,10 @@ class ManagerTest extends \Test\TestCase { ->willReturn(false); $file->method('getStorage') ->willReturn($storage); + $file->method('getId')->willReturn(108); $node->method('getStorage') ->willReturn($storage); + $node->method('getId')->willReturn(108); $data = [ [$this->createShare(null, IShare::TYPE_USER, $file, null, $user0, $user0, 31, null, null), 'SharedWith is not a valid user', true], @@ -679,6 +684,7 @@ class ManagerTest extends \Test\TestCase { ]; $nonShareAble = $this->createMock(Folder::class); + $nonShareAble->method('getId')->willReturn(108); $nonShareAble->method('isShareable')->willReturn(false); $nonShareAble->method('getPath')->willReturn('path'); $nonShareAble->method('getName')->willReturn('name'); @@ -714,16 +720,22 @@ class ManagerTest extends \Test\TestCase { $data[] = [$this->createShare(null, IShare::TYPE_GROUP, $limitedPermssions, $group0, $user0, $user0, 17, null, null), 'Cannot increase permissions of path', true]; $data[] = [$this->createShare(null, IShare::TYPE_LINK, $limitedPermssions, null, $user0, $user0, 3, null, null), 'Cannot increase permissions of path', true]; + $nonMovableStorage = $this->createMock(Storage\IStorage::class); + $nonMovableStorage->method('instanceOfStorage') + ->with('\OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $nonMovableStorage->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_ALL); $nonMoveableMountPermssions = $this->createMock(Folder::class); $nonMoveableMountPermssions->method('isShareable')->willReturn(true); $nonMoveableMountPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ); $nonMoveableMountPermssions->method('getId')->willReturn(108); $nonMoveableMountPermssions->method('getPath')->willReturn('path'); $nonMoveableMountPermssions->method('getName')->willReturn('name'); + $nonMoveableMountPermssions->method('getInternalPath')->willReturn(''); $nonMoveableMountPermssions->method('getOwner') ->willReturn($owner); $nonMoveableMountPermssions->method('getStorage') - ->willReturn($storage); + ->willReturn($nonMovableStorage); $data[] = [$this->createShare(null, IShare::TYPE_USER, $nonMoveableMountPermssions, $user2, $user0, $user0, 11, null, null), 'Cannot increase permissions of path', false]; $data[] = [$this->createShare(null, IShare::TYPE_GROUP, $nonMoveableMountPermssions, $group0, $user0, $user0, 11, null, null), 'Cannot increase permissions of path', false]; @@ -797,10 +809,9 @@ class ManagerTest extends \Test\TestCase { ->method('getId') ->willReturn(42); // Id 108 is used in the data to refer to the node of the share. - $userFolder->expects($this->any()) - ->method('getFirstNodeById') + $userFolder->method('getById') ->with(108) - ->willReturn($share->getNode()); + ->willReturn([$share->getNode()]); $userFolder->expects($this->any()) ->method('getRelativePath') ->willReturnArgument(0); |