Commit 80f83ab5 authored by Vincent Petry's avatar Vincent Petry
Browse files

Do not return shares for files outside "files"

parent 7ad4dfa2
...@@ -327,18 +327,20 @@ class Share extends \OC\Share\Constants { ...@@ -327,18 +327,20 @@ class Share extends \OC\Share\Constants {
*/ */
public static function getItemSharedWithUser($itemType, $itemSource, $user, $owner = null, $shareType = null) { public static function getItemSharedWithUser($itemType, $itemSource, $user, $owner = null, $shareType = null) {
$shares = array(); $shares = array();
$fileDependend = false; $fileDependent = false;
if ($itemType === 'file' || $itemType === 'folder') { if ($itemType === 'file' || $itemType === 'folder') {
$fileDependend = true; $fileDependent = true;
$column = 'file_source'; $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 { } else {
$column = 'item_source'; $column = 'item_source';
$where = 'WHERE'; $where = 'WHERE';
} }
$select = self::createSelectStatement(self::FORMAT_NONE, $fileDependend); $select = self::createSelectStatement(self::FORMAT_NONE, $fileDependent);
$where .= ' `' . $column . '` = ? AND `item_type` = ? '; $where .= ' `' . $column . '` = ? AND `item_type` = ? ';
$arguments = array($itemSource, $itemType); $arguments = array($itemSource, $itemType);
...@@ -363,6 +365,9 @@ class Share extends \OC\Share\Constants { ...@@ -363,6 +365,9 @@ class Share extends \OC\Share\Constants {
$result = \OC_DB::executeAudited($query, $arguments); $result = \OC_DB::executeAudited($query, $arguments);
while ($row = $result->fetchRow()) { while ($row = $result->fetchRow()) {
if ($fileDependent && !self::isFileReachable($row['path'], $row['storage_id'])) {
continue;
}
$shares[] = $row; $shares[] = $row;
} }
...@@ -1382,10 +1387,11 @@ class Share extends \OC\Share\Constants { ...@@ -1382,10 +1387,11 @@ class Share extends \OC\Share\Constants {
} else { } else {
$root = ''; $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)) { 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; $fileDependent = true;
$queryArgs = array(); $queryArgs = array();
} else { } else {
...@@ -1526,6 +1532,9 @@ class Share extends \OC\Share\Constants { ...@@ -1526,6 +1532,9 @@ class Share extends \OC\Share\Constants {
while ($row = $result->fetchRow()) { while ($row = $result->fetchRow()) {
self::transformDBResults($row); self::transformDBResults($row);
// Filter out duplicate group shares for users with unique targets // 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']])) { if ($row['share_type'] == self::$shareTypeGroupUserUnique && isset($items[$row['parent']])) {
$row['share_type'] = self::SHARE_TYPE_GROUP; $row['share_type'] = self::SHARE_TYPE_GROUP;
$row['unique_name'] = true; // remember that we use a unique name for this user $row['unique_name'] = true; // remember that we use a unique name for this user
...@@ -2181,7 +2190,9 @@ class Share extends \OC\Share\Constants { ...@@ -2181,7 +2190,9 @@ class Share extends \OC\Share\Constants {
$select = '*'; $select = '*';
if ($format == self::FORMAT_STATUSES) { if ($format == self::FORMAT_STATUSES) {
if ($fileDependent) { 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 { } else {
$select = '`id`, `parent`, `share_type`, `share_with`, `uid_owner`, `item_source`, `stime`, `*PREFIX*share`.`permissions`'; $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 { ...@@ -2190,7 +2201,8 @@ class Share extends \OC\Share\Constants {
if ($fileDependent) { if ($fileDependent) {
$select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `*PREFIX*share`.`parent`,' $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `*PREFIX*share`.`parent`,'
. ' `share_type`, `share_with`, `file_source`, `file_target`, `path`, `*PREFIX*share`.`permissions`, `stime`,' . ' `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 { } else {
$select = '`id`, `item_type`, `item_source`, `parent`, `share_type`, `share_with`, `*PREFIX*share`.`permissions`,' $select = '`id`, `item_type`, `item_source`, `parent`, `share_type`, `share_with`, `*PREFIX*share`.`permissions`,'
. ' `stime`, `file_source`, `expiration`, `token`, `mail_send`, `uid_owner`'; . ' `stime`, `file_source`, `expiration`, `token`, `mail_send`, `uid_owner`';
...@@ -2203,9 +2215,11 @@ class Share extends \OC\Share\Constants { ...@@ -2203,9 +2215,11 @@ class Share extends \OC\Share\Constants {
. '`*PREFIX*share`.`permissions`, `expiration`, `storage`, `*PREFIX*filecache`.`parent` as `file_parent`, ' . '`*PREFIX*share`.`permissions`, `expiration`, `storage`, `*PREFIX*filecache`.`parent` as `file_parent`, '
. '`name`, `mtime`, `mimetype`, `mimepart`, `size`, `unencrypted_size`, `encrypted`, `etag`, `mail_send`'; . '`name`, `mtime`, `mimetype`, `mimepart`, `size`, `unencrypted_size`, `encrypted`, `etag`, `mail_send`';
} else { } else {
$select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `item_target`, $select = '`*PREFIX*share`.`id`, `item_type`, `item_source`, `item_target`,'
`*PREFIX*share`.`parent`, `share_type`, `share_with`, `uid_owner`, . '`*PREFIX*share`.`parent`, `share_type`, `share_with`, `uid_owner`,'
`file_source`, `path`, `file_target`, `*PREFIX*share`.`permissions`, `stime`, `expiration`, `token`, `storage`, `mail_send`'; . '`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 { ...@@ -2391,4 +2405,27 @@ class Share extends \OC\Share\Constants {
return (int)\OCP\Config::getAppValue('core', 'shareapi_expire_after_n_days', '7'); 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;
}
} }
...@@ -388,6 +388,45 @@ class Test_Share extends \Test\TestCase { ...@@ -388,6 +388,45 @@ class Test_Share extends \Test\TestCase {
$this->assertSame(\OCP\Share::SHARE_TYPE_USER, $share['share_type']); $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() { public function testSetExpireDateInPast() {
OC_User::setUserId($this->user1); OC_User::setUserId($this->user1);
$this->shareUserOneTestFileWithUserTwo(); $this->shareUserOneTestFileWithUserTwo();
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment