summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--lib/private/Share20/Manager.php57
-rw-r--r--tests/lib/Share20/ManagerTest.php23
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);