take permissions from multiple paths into account for share permissions

Signed-off-by: Robin Appelman <robin@icewind.nl>

diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
index 6375ee3d7030f9dd807a7df861ef2ed2f46dfa41..a29f8f6a2b2e3f92c74e54bf45e97ae626a75862 100644 (file)
--- a/apps/files_sharing/lib/Controller/ShareAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -44,6 +44,7 @@ declare(strict_types=1);
  */
 namespace OCA\Files_Sharing\Controller;
 
+use OC\Files\FileInfo;
 use OCA\Files_Sharing\Exceptions\SharingRightsException;
 use OCA\Files_Sharing\External\Storage;
 use OCA\Files\Helper;
@@ -469,12 +470,22 @@ class ShareAPIController extends OCSController {
 
                $userFolder = $this->rootFolder->getUserFolder($this->currentUser);
                try {
-                       $path = $userFolder->get($path);
+                       /** @var \OC\Files\Node\Node $node */
+                       $node = $userFolder->get($path);
                } catch (NotFoundException $e) {
                        throw new OCSNotFoundException($this->l->t('Wrong path, file/folder does not exist'));
                }
 
-               $share->setNode($path);
+               // a user can have access to a file through different paths, with differing permissions
+               // combine all permissions to determine if the user can share this file
+               $nodes = $userFolder->getById($node->getId());
+               foreach ($nodes as $nodeById) {
+                       /** @var FileInfo $fileInfo */
+                       $fileInfo = $node->getFileInfo();
+                       $fileInfo['permissions'] |= $nodeById->getPermissions();
+               }
+
+               $share->setNode($node);
 
                try {
                        $this->lock($share->getNode());
@@ -489,7 +500,7 @@ class ShareAPIController extends OCSController {
                // Shares always require read permissions
                $permissions |= Constants::PERMISSION_READ;
 
-               if ($path instanceof \OCP\Files\File) {
+               if ($node instanceof \OCP\Files\File) {
                        // Single file shares should never have delete or create permissions
                        $permissions &= ~Constants::PERMISSION_DELETE;
                        $permissions &= ~Constants::PERMISSION_CREATE;
@@ -500,8 +511,8 @@ class ShareAPIController extends OCSController {
                 * We check the permissions via webdav. But the permissions of the mount point
                 * do not equal the share permissions. Here we fix that for federated mounts.
                 */
-               if ($path->getStorage()->instanceOfStorage(Storage::class)) {
-                       $permissions &= ~($permissions & ~$path->getPermissions());
+               if ($node->getStorage()->instanceOfStorage(Storage::class)) {
+                       $permissions &= ~($permissions & ~$node->getPermissions());
                }
 
                if ($shareType === IShare::TYPE_USER) {
@@ -537,7 +548,7 @@ class ShareAPIController extends OCSController {
                                }
 
                                // Public upload can only be set for folders
-                               if ($path instanceof \OCP\Files\File) {
+                               if ($node instanceof \OCP\Files\File) {
                                        throw new OCSNotFoundException($this->l->t('Public upload is only possible for publicly shared folders'));
                                }
 
@@ -573,7 +584,7 @@ class ShareAPIController extends OCSController {
 
                        if ($sendPasswordByTalk === 'true') {
                                if (!$this->appManager->isEnabledForUser('spreed')) {
-                                       throw new OCSForbiddenException($this->l->t('Sharing %s sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled', [$path->getPath()]));
+                                       throw new OCSForbiddenException($this->l->t('Sharing %s sending the password by Nextcloud Talk failed because Nextcloud Talk is not enabled', [$node->getPath()]));
                                }
 
                                $share->setSendPasswordByTalk(true);
@@ -590,7 +601,7 @@ class ShareAPIController extends OCSController {
                        }
                } elseif ($shareType === IShare::TYPE_REMOTE) {
                        if (!$this->shareManager->outgoingServer2ServerSharesAllowed()) {
-                               throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$path->getPath(), $shareType]));
+                               throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$node->getPath(), $shareType]));
                        }
 
                        if ($shareWith === null) {
@@ -609,7 +620,7 @@ class ShareAPIController extends OCSController {
                        }
                } elseif ($shareType === IShare::TYPE_REMOTE_GROUP) {
                        if (!$this->shareManager->outgoingServer2ServerGroupSharesAllowed()) {
-                               throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$path->getPath(), $shareType]));
+                               throw new OCSForbiddenException($this->l->t('Sharing %1$s failed because the back end does not allow shares from type %2$s', [$node->getPath(), $shareType]));
                        }
 
                        if ($shareWith === null) {
@@ -643,13 +654,13 @@ class ShareAPIController extends OCSController {
                        try {
                                $this->getRoomShareHelper()->createShare($share, $shareWith, $permissions, $expireDate);
                        } catch (QueryException $e) {
-                               throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$path->getPath()]));
+                               throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$node->getPath()]));
                        }
                } elseif ($shareType === IShare::TYPE_DECK) {
                        try {
                                $this->getDeckShareHelper()->createShare($share, $shareWith, $permissions, $expireDate);
                        } catch (QueryException $e) {
-                               throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$path->getPath()]));
+                               throw new OCSForbiddenException($this->l->t('Sharing %s failed because the back end does not support room shares', [$node->getPath()]));
                        }
                } else {
                        throw new OCSBadRequestException($this->l->t('Unknown share type'));

diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
index 324862a746da4283e05289489e05ce6a49427c85..2da8d600e2441bd43f98ac5c19460fa9f519cfb7 100644 (file)
--- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
@@ -1619,6 +1619,8 @@ class ShareAPIControllerTest extends TestCase {
                                ->method('get')
                                ->with('valid-path')
                                ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $path->expects($this->once())
                        ->method('lock')
@@ -1651,6 +1653,8 @@ class ShareAPIControllerTest extends TestCase {
                        ->method('get')
                        ->with('valid-path')
                        ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $path->expects($this->once())
                        ->method('lock')
@@ -1683,6 +1687,8 @@ class ShareAPIControllerTest extends TestCase {
                        ->method('get')
                        ->with('valid-path')
                        ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
                $path->expects($this->once())
                        ->method('lock')
                        ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED);
@@ -1733,6 +1739,8 @@ class ShareAPIControllerTest extends TestCase {
                                ->method('get')
                                ->with('valid-path')
                                ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $this->userManager->method('userExists')->with('validUser')->willReturn(true);
 
@@ -1787,6 +1795,8 @@ class ShareAPIControllerTest extends TestCase {
                                ->method('get')
                                ->with('valid-path')
                                ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $path->expects($this->once())
                        ->method('lock')
@@ -1844,6 +1854,8 @@ class ShareAPIControllerTest extends TestCase {
                        ->method('get')
                        ->with('valid-path')
                        ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $this->groupManager->method('groupExists')->with('validGroup')->willReturn(true);
 
@@ -1896,6 +1908,8 @@ class ShareAPIControllerTest extends TestCase {
                        ->method('get')
                        ->with('valid-path')
                        ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $this->groupManager->method('groupExists')->with('validGroup')->willReturn(true);
 
@@ -1926,6 +1940,8 @@ class ShareAPIControllerTest extends TestCase {
                $path->method('getStorage')->willReturn($storage);
                $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
                $this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+               $this->rootFolder->method('getById')
+                       ->willReturn([]);
 
                $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
 
@@ -1945,6 +1961,8 @@ class ShareAPIControllerTest extends TestCase {
                $path->method('getStorage')->willReturn($storage);
                $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
                $this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+               $this->rootFolder->method('getById')
+                       ->willReturn([]);
 
                $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
                $this->shareManager->method('shareApiAllowLinks')->willReturn(true);
@@ -1965,6 +1983,8 @@ class ShareAPIControllerTest extends TestCase {
                $path->method('getStorage')->willReturn($storage);
                $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
                $this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+               $this->rootFolder->method('getById')
+                       ->willReturn([]);
 
                $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
                $this->shareManager->method('shareApiAllowLinks')->willReturn(true);
@@ -1984,6 +2004,8 @@ class ShareAPIControllerTest extends TestCase {
                $path->method('getStorage')->willReturn($storage);
                $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
                $this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+               $this->rootFolder->method('getById')
+                       ->willReturn([]);
 
                $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
                $this->shareManager->method('shareApiAllowLinks')->willReturn(true);
@@ -2018,6 +2040,8 @@ class ShareAPIControllerTest extends TestCase {
                $path->method('getStorage')->willReturn($storage);
                $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
                $this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+               $this->rootFolder->method('getById')
+                       ->willReturn([]);
 
                $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
                $this->shareManager->method('shareApiAllowLinks')->willReturn(true);
@@ -2052,6 +2076,8 @@ class ShareAPIControllerTest extends TestCase {
                $path->method('getStorage')->willReturn($storage);
                $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
                $this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+               $this->rootFolder->method('getById')
+                       ->willReturn([]);
 
                $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
                $this->shareManager->method('shareApiAllowLinks')->willReturn(true);
@@ -2094,6 +2120,8 @@ class ShareAPIControllerTest extends TestCase {
                $path->method('getPath')->willReturn('valid-path');
                $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
                $this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+               $this->rootFolder->method('getById')
+                       ->willReturn([]);
 
                $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
                $this->shareManager->method('shareApiAllowLinks')->willReturn(true);
@@ -2127,6 +2155,8 @@ class ShareAPIControllerTest extends TestCase {
                $path->method('getStorage')->willReturn($storage);
                $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
                $this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+               $this->rootFolder->method('getById')
+                       ->willReturn([]);
 
                $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
                $this->shareManager->method('shareApiAllowLinks')->willReturn(true);
@@ -2168,6 +2198,8 @@ class ShareAPIControllerTest extends TestCase {
                $path->method('getStorage')->willReturn($storage);
                $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
                $this->rootFolder->method('get')->with('valid-path')->willReturn($path);
+               $this->rootFolder->method('getById')
+                       ->willReturn([]);
 
                $this->shareManager->method('newShare')->willReturn(\OC::$server->getShareManager()->newShare());
                $this->shareManager->method('shareApiAllowLinks')->willReturn(true);
@@ -2216,6 +2248,8 @@ class ShareAPIControllerTest extends TestCase {
                                ->method('get')
                                ->with('valid-path')
                                ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $this->userManager->method('userExists')->with('validUser')->willReturn(true);
 
@@ -2286,6 +2320,8 @@ class ShareAPIControllerTest extends TestCase {
                                ->method('get')
                                ->with('valid-path')
                                ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $this->userManager->method('userExists')->with('validUser')->willReturn(true);
 
@@ -2338,6 +2374,8 @@ class ShareAPIControllerTest extends TestCase {
                                ->method('get')
                                ->with('valid-path')
                                ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $path->expects($this->once())
                        ->method('lock')
@@ -2421,6 +2459,8 @@ class ShareAPIControllerTest extends TestCase {
                                ->method('get')
                                ->with('valid-path')
                                ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $path->expects($this->once())
                        ->method('lock')
@@ -2461,6 +2501,8 @@ class ShareAPIControllerTest extends TestCase {
                                ->method('get')
                                ->with('valid-path')
                                ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $path->expects($this->once())
                        ->method('lock')
@@ -2541,6 +2583,8 @@ class ShareAPIControllerTest extends TestCase {
                        ->method('get')
                        ->with('valid-path')
                        ->willReturn($path);
+               $userFolder->method('getById')
+                       ->willReturn([]);
 
                $this->userManager->method('userExists')->with('validUser')->willReturn(true);