Use the correct mountpoint to calculate

If we use the owners mount point this results in null. And then the rest
of the checks get called with null. Which doesn't work.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index 24aa54ea0d6035191097876ee572dab97221830b..3b022f5951c13dfd7604cca0d0f6652ba7b5ffcd 100644 (file)
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -298,13 +298,20 @@ class Manager implements IManager {
 
                $isFederatedShare = $share->getNode()->getStorage()->instanceOfStorage('\OCA\Files_Sharing\External\Storage');
                $permissions = 0;
-               $mount = $share->getNode()->getMountPoint();
+
+               $userMounts = $userFolder->getById($share->getNode()->getId());
+               $userMount = array_shift($userMounts);
+               $mount = $userMount->getMountPoint();
                if (!$isFederatedShare && $share->getNode()->getOwner() && $share->getNode()->getOwner()->getUID() !== $share->getSharedBy()) {
                        // When it's a reshare use the parent share permissions as maximum
                        $userMountPointId = $mount->getStorageRootId();
                        $userMountPoints = $userFolder->getById($userMountPointId);
                        $userMountPoint = array_shift($userMountPoints);
 
+                       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));

diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
index 65a5ff8946d67db4d2e201dbc3369c6248b07703..58f96c418b8d9d8671ee5802d7db2de560a8827e 100644 (file)
--- a/tests/lib/Share20/ManagerTest.php
+++ b/tests/lib/Share20/ManagerTest.php
@@ -613,6 +613,7 @@ class ManagerTest extends \Test\TestCase {
                $limitedPermssions = $this->createMock(File::class);
                $limitedPermssions->method('isShareable')->willReturn(true);
                $limitedPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ);
+               $limitedPermssions->method('getId')->willReturn(108);
                $limitedPermssions->method('getPath')->willReturn('path');
                $limitedPermssions->method('getOwner')
                        ->willReturn($owner);
@@ -634,6 +635,7 @@ class ManagerTest extends \Test\TestCase {
                $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('getOwner')
                        ->willReturn($owner);
@@ -655,6 +657,7 @@ class ManagerTest extends \Test\TestCase {
                $allPermssions = $this->createMock(Folder::class);
                $allPermssions->method('isShareable')->willReturn(true);
                $allPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_ALL);
+               $allPermssions->method('getId')->willReturn(108);
                $allPermssions->method('getOwner')
                        ->willReturn($owner);
                $allPermssions->method('getStorage')
@@ -675,6 +678,7 @@ class ManagerTest extends \Test\TestCase {
                $remoteFile = $this->createMock(Folder::class);
                $remoteFile->method('isShareable')->willReturn(true);
                $remoteFile->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ ^ \OCP\Constants::PERMISSION_UPDATE);
+               $remoteFile->method('getId')->willReturn(108);
                $remoteFile->method('getOwner')
                        ->willReturn($owner);
                $remoteFile->method('getStorage')
@@ -709,6 +713,11 @@ class ManagerTest extends \Test\TestCase {
                $userFolder->expects($this->any())
                        ->method('getId')
                        ->willReturn(42);
+               // Id 108 is used in the data to refer to the node of the share.
+               $userFolder->expects($this->any())
+                       ->method('getById')
+                       ->with(108)
+                       ->willReturn([$share->getNode()]);
                $userFolder->expects($this->any())
                        ->method('getRelativePath')
                        ->willReturnArgument(0);