Do not return shares for files outside "files"

diff --git a/lib/private/share/share.php b/lib/private/share/share.php
index aeb9a9a47675b26f6d86e5cb7f8cd409b3797bc0..9500ec69428156cb231ad0c3d7e8016cbb6e2e9c 100644 (file)
--- a/lib/private/share/share.php
+++ b/lib/private/share/share.php
@@ -327,18 +327,20 @@ class Share extends \OC\Share\Constants {
         */
        public static function getItemSharedWithUser($itemType, $itemSource, $user, $owner = null, $shareType = null) {
                $shares = array();
-               $fileDependend = false;
+               $fileDependent = false;
 
                if ($itemType === 'file' || $itemType === 'folder') {
-                       $fileDependend = true;
+                       $fileDependent = true;
                        $column = 'file_source';
-                       $where = 'INNER JOIN `*PREFIX*filecache` ON `file_source` = `*PREFIX*filecache`.`fileid` WHERE';
+                       $where = 'INNER JOIN `*PREFIX*filecache` ON `file_source` = `*PREFIX*filecache`.`fileid` ';
+                       $where .= 'INNER JOIN `*PREFIX*storages` ON `numeric_id` = `*PREFIX*filecache`.`storage` ';
+                       $where .= ' WHERE';
                } else {
                        $column = 'item_source';
                        $where = 'WHERE';
                }
 
-               $select = self::createSelectStatement(self::FORMAT_NONE, $fileDependend);
+               $select = self::createSelectStatement(self::FORMAT_NONE, $fileDependent);
 
                $where .= ' `' . $column . '` = ? AND `item_type` = ? ';
                $arguments = array($itemSource, $itemType);
@@ -363,6 +365,9 @@ class Share extends \OC\Share\Constants {
                $result = \OC_DB::executeAudited($query, $arguments);
 
                while ($row = $result->fetchRow()) {
+                       if ($fileDependent && !self::isFileReachable($row['path'], $row['storage_id'])) {
+                               continue;
+                       }
                        $shares[] = $row;
                }
 
@@ -1382,10 +1387,11 @@ class Share extends \OC\Share\Constants {
                        } else {
                                $root = '';
                        }
-                       $where = 'INNER JOIN `*PREFIX*filecache` ON `file_source` = `*PREFIX*filecache`.`fileid`';
+                       $where = 'INNER JOIN `*PREFIX*filecache` ON `file_source` = `*PREFIX*filecache`.`fileid` ';
                        if (!isset($item)) {
-                               $where .= ' WHERE `file_target` IS NOT NULL';
+                               $where .= ' AND `file_target` IS NOT NULL ';
                        }
+                       $where .= 'INNER JOIN `*PREFIX*storages` ON `numeric_id` = `*PREFIX*filecache`.`storage` ';
                        $fileDependent = true;
                        $queryArgs = array();
                } else {
@@ -1526,6 +1532,9 @@ class Share extends \OC\Share\Constants {
                while ($row = $result->fetchRow()) {
                        self::transformDBResults($row);
                        // Filter out duplicate group shares for users with unique targets
+                       if ($fileDependent && !self::isFileReachable($row['path'], $row['storage_id'])) {
+                               continue;
+                       }
                        if ($row['share_type'] == self::$shareTypeGroupUserUnique && isset($items[$row['parent']])) {
                                $row['share_type'] = self::SHARE_TYPE_GROUP;
                                $row['unique_name'] = true; // remember that we use a unique name for this user
@@ -2181,7 +2190,9 @@ class Share extends \OC\Share\Constants {
                $select = '*';
                if ($format == self::FORMAT_STATUSES) {
                        if ($fileDependent) {
-                               $select = '`*PREFIX*share`.`id`, `*PREFIX*share`.`parent`, `share_type`, `path`, `storage`, `share_with`, `uid_owner` , `file_source`, `stime`, `*PREFIX*share`.`permissions`';
+                               $select = '`*PREFIX*share`.`id`, `*PREFIX*share`.`parent`, `share_type`, `path`, `storage`, '
+                                       . '`share_with`, `uid_owner` , `file_source`, `stime`, `*PREFIX*share`.`permissions`, '
+                                       . '`*PREFIX*storages`.`id` AS `storage_id`';
                        } else {
                                $select = '`id`, `parent`, `share_type`, `share_with`, `uid_owner`, `item_source`, `stime`, `*PREFIX*share`.`permissions`';
                        }
@@ -2190,7 +2201,8 @@ class Share extends \OC\Share\Constants {
                                if ($fileDependent) {
                                        $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `*PREFIX*share`.`parent`,'
                                                . ' `share_type`, `share_with`, `file_source`, `file_target`, `path`, `*PREFIX*share`.`permissions`, `stime`,'
-                                               . ' `expiration`, `token`, `storage`, `mail_send`, `uid_owner`';
+                                               . ' `expiration`, `token`, `storage`, `mail_send`, `uid_owner`, '
+                                               . '`*PREFIX*storages`.`id` AS `storage_id`';
                                } else {
                                        $select = '`id`, `item_type`, `item_source`, `parent`, `share_type`, `share_with`, `*PREFIX*share`.`permissions`,'
                                                . ' `stime`, `file_source`, `expiration`, `token`, `mail_send`, `uid_owner`';
@@ -2203,9 +2215,11 @@ class Share extends \OC\Share\Constants {
                                                        . '`*PREFIX*share`.`permissions`, `expiration`, `storage`, `*PREFIX*filecache`.`parent` as `file_parent`, '
                                                        . '`name`, `mtime`, `mimetype`, `mimepart`, `size`, `unencrypted_size`, `encrypted`, `etag`, `mail_send`';
                                        } else {
-                                               $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `item_target`,
-                                                       `*PREFIX*share`.`parent`, `share_type`, `share_with`, `uid_owner`,
-                                                       `file_source`, `path`, `file_target`, `*PREFIX*share`.`permissions`, `stime`, `expiration`, `token`, `storage`, `mail_send`';
+                                               $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `item_target`,'
+                                                       . '`*PREFIX*share`.`parent`, `share_type`, `share_with`, `uid_owner`,'
+                                                       . '`file_source`, `path`, `file_target`, `*PREFIX*share`.`permissions`,'
+                                                   . '`stime`, `expiration`, `token`, `storage`, `mail_send`,'
+                                                       . '`*PREFIX*storages`.`id` AS `storage_id`';
                                        }
                                }
                        }
@@ -2391,4 +2405,27 @@ class Share extends \OC\Share\Constants {
                return (int)\OCP\Config::getAppValue('core', 'shareapi_expire_after_n_days', '7');
        }
 
+       /**
+        * Checks whether the given path is reachable for the given owner
+        *
+        * @param string $path path relative to files
+        * @param string $ownerStorageId storage id of the owner
+        *
+        * @return boolean true if file is reachable, false otherwise
+        */
+       private static function isFileReachable($path, $ownerStorageId) {
+               // if outside the home storage, file is always considered reachable
+               if (!(substr($ownerStorageId, 0, 6) === 'home::')) {
+                       return true;
+               }
+
+               // if inside the home storage, the file has to be under "/files/"
+               $path = ltrim($path, '/');
+               if (substr($path, 0, 6) === 'files/') {
+                       return true;
+               }
+
+               return false;
+       }
+
 }

diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php
index 42bb82968af117304caec7407018199b6d62023f..2bc1e320e5749d8dd1f26b31db04f668543fb97d 100644 (file)
--- a/tests/lib/share/share.php
+++ b/tests/lib/share/share.php
@@ -388,6 +388,45 @@ class Test_Share extends \Test\TestCase {
                $this->assertSame(\OCP\Share::SHARE_TYPE_USER, $share['share_type']);
        }
 
+       public function testGetShareFromOutsideFilesFolder() {
+               OC_User::setUserId($this->user1);
+               $view = new \OC\Files\View('/' . $this->user1 . '/');
+               $view->mkdir('files/test');
+               $view->mkdir('files/test/sub');
+
+               $view->mkdir('files_trashbin');
+               $view->mkdir('files_trashbin/files');
+
+               $fileInfo = $view->getFileInfo('files/test/sub');
+               $fileId = $fileInfo->getId();
+
+               $this->assertTrue(
+                       OCP\Share::shareItem('folder', $fileId, OCP\Share::SHARE_TYPE_USER, $this->user2, \OCP\Constants::PERMISSION_READ),
+                       'Failed asserting that user 1 successfully shared "test/sub" with user 2.'
+               );
+
+               $result = OCP\Share::getItemShared('folder', $fileId, Test_Share_Backend::FORMAT_SOURCE);
+               $this->assertNotEmpty($result);
+
+               $result = OCP\Share::getItemSharedWithUser('folder', $fileId, $this->user2);
+               $this->assertNotEmpty($result);
+
+               $result = OCP\Share::getItemsSharedWithUser('folder', $this->user2);
+               $this->assertNotEmpty($result);
+
+               // move to trash (keeps file id)
+               $view->rename('files/test', 'files_trashbin/files/test');
+
+               $result = OCP\Share::getItemShared('folder', $fileId, Test_Share_Backend::FORMAT_SOURCE);
+               $this->assertEmpty($result, 'Share must not be returned for files outside of "files"');
+
+               $result = OCP\Share::getItemSharedWithUser('folder', $fileId, $this->user2);
+               $this->assertEmpty($result, 'Share must not be returned for files outside of "files"');
+
+               $result = OCP\Share::getItemsSharedWithUser('folder', $this->user2);
+               $this->assertEmpty($result, 'Share must not be returned for files outside of "files"');
+       }
+
        public function testSetExpireDateInPast() {
                OC_User::setUserId($this->user1);
                $this->shareUserOneTestFileWithUserTwo();