From cfc52ccc3d3e7e233912adc58bdc95618d941a30 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 28 Mar 2014 15:24:13 +0100 Subject: [PATCH 01/34] add some action items --- lib/private/share/constants.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/private/share/constants.php b/lib/private/share/constants.php index 7e4223d10f..4c398c43c2 100644 --- a/lib/private/share/constants.php +++ b/lib/private/share/constants.php @@ -26,13 +26,13 @@ class Constants { const SHARE_TYPE_USER = 0; const SHARE_TYPE_GROUP = 1; const SHARE_TYPE_LINK = 3; - const SHARE_TYPE_EMAIL = 4; - const SHARE_TYPE_CONTACT = 5; - const SHARE_TYPE_REMOTE = 6; + const SHARE_TYPE_EMAIL = 4; // ToDo Check if it is still in use otherwise remove it + const SHARE_TYPE_CONTACT = 5; // ToDo Check if it is still in use otherwise remove it + const SHARE_TYPE_REMOTE = 6; // ToDo Check if it is still in use otherwise remove it const FORMAT_NONE = -1; const FORMAT_STATUSES = -2; - const FORMAT_SOURCES = -3; + const FORMAT_SOURCES = -3; // ToDo Check if it is still in use otherwise remove it const TOKEN_LENGTH = 32; // see db_structure.xml From a27db9e4cadb9b5ee26f7b17d3a5997b07639489 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 2 Apr 2014 12:04:51 +0200 Subject: [PATCH 02/34] first steps to remove the shared folder: - mount shares to the root folder instead of "Shared/" - navigate in shared folder and sub-folders - show previews - show correct file permissions - download/edit files --- apps/files_sharing/lib/cache.php | 48 +++++++-------- apps/files_sharing/lib/share/file.php | 75 +++++++++++++----------- apps/files_sharing/lib/sharedstorage.php | 60 +++++++++++++------ 3 files changed, 108 insertions(+), 75 deletions(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index eeb62c3cce..c509ec2457 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -2,8 +2,9 @@ /** * ownCloud * - * @author Michael Gapczynski - * @copyright 2012 Michael Gapczynski mtgap@owncloud.com + * @author Bjoern Schiessle, Michael Gapczynski + * @copyright 2012 Michael Gapczynski + * 2014 Bjoern Schiessle * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE @@ -46,7 +47,10 @@ class Shared_Cache extends Cache { * @return \OC\Files\Cache\Cache */ private function getSourceCache($target) { - $source = \OC_Share_Backend_File::getSource($target); + if ($target === false) { + $target = ''; + } + $source = \OC_Share_Backend_File::getSource($target, $this->storage->getMountPoint(), $this->storage->getShareType()); if (isset($source['path']) && isset($source['fileOwner'])) { \OC\Files\Filesystem::initMountPoints($source['fileOwner']); $mount = \OC\Files\Filesystem::getMountByNumericId($source['storage']); @@ -127,28 +131,24 @@ class Shared_Cache extends Cache { * @return array */ public function getFolderContents($folder) { - if ($folder == '') { - $files = \OCP\Share::getItemsSharedWith('file', \OC_Share_Backend_File::FORMAT_GET_FOLDER_CONTENTS); - foreach ($files as &$file) { - $file['mimetype'] = $this->getMimetype($file['mimetype']); - $file['mimepart'] = $this->getMimetype($file['mimepart']); - $file['usersPath'] = 'files/Shared/' . ltrim($file['path'], '/'); - } - return $files; - } else { - $cache = $this->getSourceCache($folder); - if ($cache) { - $parent = $this->storage->getFile($folder); - $sourceFolderContent = $cache->getFolderContents($this->files[$folder]); - foreach ($sourceFolderContent as $key => $c) { - $sourceFolderContent[$key]['usersPath'] = 'files/Shared/' . $folder . '/' . $c['name']; - $sourceFolderContent[$key]['uid_owner'] = $parent['uid_owner']; - $sourceFolderContent[$key]['displayname_owner'] = $parent['uid_owner']; - } - return $sourceFolderContent; - } + if ($folder === false) { + $folder = ''; } + + $cache = $this->getSourceCache($folder); + if ($cache) { + $parent = $this->storage->getFile($folder); + $sourceFolderContent = $cache->getFolderContents($this->files[$folder]); + foreach ($sourceFolderContent as $key => $c) { + $sourceFolderContent[$key]['usersPath'] = 'files/' . $folder . '/' . $c['name']; + $sourceFolderContent[$key]['uid_owner'] = $parent['uid_owner']; + $sourceFolderContent[$key]['displayname_owner'] = $parent['uid_owner']; + } + + return $sourceFolderContent; + } + return false; } @@ -214,7 +214,7 @@ class Shared_Cache extends Cache { */ public function move($source, $target) { if ($cache = $this->getSourceCache($source)) { - $file = \OC_Share_Backend_File::getSource($target); + $file = \OC_Share_Backend_File::getSource($target, $this->storage->getMountPoint(), $this->storage->getShareType()); if ($file && isset($file['path'])) { $cache->move($this->files[$source], $file['path']); } diff --git a/apps/files_sharing/lib/share/file.php b/apps/files_sharing/lib/share/file.php index 5e00050fe1..b5196ab6fa 100644 --- a/apps/files_sharing/lib/share/file.php +++ b/apps/files_sharing/lib/share/file.php @@ -2,8 +2,9 @@ /** * ownCloud * -* @author Michael Gapczynski -* @copyright 2012 Michael Gapczynski mtgap@owncloud.com +* @author Bjoern Schiessle, Michael Gapczynski +* @copyright 2012 Michael Gapczynski + * 2014 Bjoern Schiessle * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE @@ -146,42 +147,50 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { return array(); } - public static function getSource($target) { - if ($target == '') { - return false; - } - $target = '/'.$target; - $target = rtrim($target, '/'); - $pos = strpos($target, '/', 1); - // Get shared folder name - if ($pos !== false) { - $folder = substr($target, 0, $pos); - $source = \OCP\Share::getItemSharedWith('folder', $folder, \OC_Share_Backend_File::FORMAT_SHARED_STORAGE); - if ($source) { - $source['path'] = $source['path'].substr($target, strlen($folder)); + /** + * @brief resolve reshares to return the correct source item + * @param array $source + * @return array source item + */ + protected static function resolveReshares($source) { + if (isset($source['parent'])) { + $parent = $source['parent']; + while (isset($parent)) { + $query = \OC_DB::prepare('SELECT `parent`, `uid_owner` FROM `*PREFIX*share` WHERE `id` = ?', 1); + $item = $query->execute(array($parent))->fetchRow(); + if (isset($item['parent'])) { + $parent = $item['parent']; + } else { + $fileOwner = $item['uid_owner']; + break; + } } } else { - $source = \OCP\Share::getItemSharedWith('file', $target, \OC_Share_Backend_File::FORMAT_SHARED_STORAGE); + $fileOwner = $source['uid_owner']; + } + if (isset($fileOwner)) { + $source['fileOwner'] = $fileOwner; + } else { + \OCP\Util::writeLog('files_sharing', "No owner found for reshare", \OCP\Util::ERROR); + } + + return $source; + } + + public static function getSource($target, $mountPoint, $itemType) { + + if ($itemType === 'folder') { + $source = \OCP\Share::getItemSharedWith('folder', $mountPoint, \OC_Share_Backend_File::FORMAT_SHARED_STORAGE); + if ($source && $target !== '') { + $source['path'] = $source['path'].'/'.$target; + } + } else { + $source = \OCP\Share::getItemSharedWith('file', $mountPoint, \OC_Share_Backend_File::FORMAT_SHARED_STORAGE); } if ($source) { - if (isset($source['parent'])) { - $parent = $source['parent']; - while (isset($parent)) { - $query = \OC_DB::prepare('SELECT `parent`, `uid_owner` FROM `*PREFIX*share` WHERE `id` = ?', 1); - $item = $query->execute(array($parent))->fetchRow(); - if (isset($item['parent'])) { - $parent = $item['parent']; - } else { - $fileOwner = $item['uid_owner']; - break; - } - } - } else { - $fileOwner = $source['uid_owner']; - } - $source['fileOwner'] = $fileOwner; - return $source; + return self::resolveReshares($source); } + \OCP\Util::writeLog('files_sharing', 'File source not found for: '.$target, \OCP\Util::DEBUG); return false; } diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index b922654e5e..25a05a0d1f 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -2,8 +2,9 @@ /** * ownCloud * - * @author Michael Gapczynski - * @copyright 2011 Michael Gapczynski mtgap@owncloud.com + * @author Bjoern Schiessle, Michael Gapczynski + * @copyright 2011 Michael Gapczynski + * 2014 Bjoern Schiessle * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE @@ -27,15 +28,17 @@ namespace OC\Files\Storage; */ class Shared extends \OC\Files\Storage\Common { - private $sharedFolder; + private $mountPoint; // mount point relative to data/user/files + private $type; // can be "file" or "folder" private $files = array(); public function __construct($arguments) { - $this->sharedFolder = $arguments['sharedFolder']; + $this->mountPoint = $arguments['shareTarget']; + $this->type = $arguments['shareType']; } public function getId() { - return 'shared::' . $this->sharedFolder; + return 'shared::' . $this->mountPoint; } /** @@ -48,14 +51,14 @@ class Shared extends \OC\Files\Storage\Common { if (!isset($this->files[$target])) { // Check for partial files if (pathinfo($target, PATHINFO_EXTENSION) === 'part') { - $source = \OC_Share_Backend_File::getSource(substr($target, 0, -5)); + $source = \OC_Share_Backend_File::getSource(substr($target, 0, -5), $this->getMountPoint(), $this->getShareType()); if ($source) { $source['path'] .= '.part'; // All partial files have delete permission $source['permissions'] |= \OCP\PERMISSION_DELETE; } } else { - $source = \OC_Share_Backend_File::getSource($target); + $source = \OC_Share_Backend_File::getSource($target, $this->getMountPoint(), $this->getShareType()); } $this->files[$target] = $source; } @@ -119,8 +122,8 @@ class Shared extends \OC\Files\Storage\Common { public function opendir($path) { if ($path == '' || $path == '/') { $files = \OCP\Share::getItemsSharedWith('file', \OC_Share_Backend_Folder::FORMAT_OPENDIR); - \OC\Files\Stream\Dir::register('shared', $files); - return opendir('fakedir://shared'); + \OC\Files\Stream\Dir::register($this->mountPoint, $files); + return opendir('fakedir://' . $this->mountPoint); } else if ($source = $this->getSourcePath($path)) { list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); return $storage->opendir($internalPath); @@ -180,7 +183,7 @@ class Shared extends \OC\Files\Storage\Common { public function isCreatable($path) { if ($path == '') { - return false; + return ($this->getPermissions($this->getMountPoint()) & \OCP\PERMISSION_CREATE); } return ($this->getPermissions($path) & \OCP\PERMISSION_CREATE); } @@ -246,7 +249,7 @@ class Shared extends \OC\Files\Storage\Common { $source = $this->getSourcePath($path); if ($source) { $info = array( - 'target' => $this->sharedFolder . $path, + 'target' => $this->mountPoint . $path, 'source' => $source, ); \OCP\Util::emitHook('\OC\Files\Storage\Shared', 'file_get_contents', $info); @@ -264,7 +267,7 @@ class Shared extends \OC\Files\Storage\Common { return false; } $info = array( - 'target' => $this->sharedFolder . $path, + 'target' => $this->mountPoint . $path, 'source' => $source, ); \OCP\Util::emitHook('\OC\Files\Storage\Shared', 'file_put_contents', $info); @@ -343,7 +346,7 @@ class Shared extends \OC\Files\Storage\Common { } } $info = array( - 'target' => $this->sharedFolder . $path, + 'target' => $this->mountPoint . $path, 'source' => $source, 'mode' => $mode, ); @@ -393,16 +396,37 @@ class Shared extends \OC\Files\Storage\Common { } public static function setup($options) { + $shares = \OCP\Share::getItemsSharedWith('file'); if (!\OCP\User::isLoggedIn() || \OCP\User::getUser() != $options['user'] - || \OCP\Share::getItemsSharedWith('file') + || $shares ) { - $user_dir = $options['user_dir']; - \OC\Files\Filesystem::mount('\OC\Files\Storage\Shared', - array('sharedFolder' => '/Shared'), - $user_dir . '/Shared/'); + foreach ($shares as $share) { + \OC\Files\Filesystem::mount('\OC\Files\Storage\Shared', + array( + 'shareTarget' => $share['file_target'], + 'shareType' => $share['item_type'], + ), + $options['user_dir'] . '/' . $share['file_target']); + } } } + /** + * @brief return mount point of share, relative to data/user/files + * @return string + */ + public function getMountPoint() { + return ltrim($this->mountPoint, '/'); + } + + /** + * @brief return share type, can be "file" or "folder" + * @return string + */ + public function getShareType() { + return $this->type; + } + public function hasUpdated($path, $time) { if ($path == '') { return false; From c377893f0e81dbefe8612fecd90434e47573be29 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 4 Apr 2014 16:54:02 +0200 Subject: [PATCH 03/34] make sure that we have the right permissions for the mount points --- apps/files_sharing/lib/permissions.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/permissions.php b/apps/files_sharing/lib/permissions.php index 31b7ac361a..c3ad63e2fd 100644 --- a/apps/files_sharing/lib/permissions.php +++ b/apps/files_sharing/lib/permissions.php @@ -31,7 +31,9 @@ class Shared_Permissions extends Permissions { */ public function get($fileId, $user) { if ($fileId == -1) { - return \OCP\PERMISSION_READ; + // if we ask for the mount point return -1 so that we can get the correct + // permissions by the path, with the root fileId we have no idea which share is meant + return -1; } $source = \OCP\Share::getItemSharedWithBySource('file', $fileId, \OC_Share_Backend_File::FORMAT_SHARED_STORAGE, null, true); From b02b6d3c236bb015e41447c686acf7167ad037e9 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 4 Apr 2014 17:45:53 +0200 Subject: [PATCH 04/34] no exception for the root of the mount point (formerly Shared/), just get the file cache information from the cache --- apps/files_sharing/lib/cache.php | 11 +---------- apps/files_sharing/lib/share/file.php | 16 ---------------- 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index c509ec2457..becd436f79 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -84,16 +84,7 @@ class Shared_Cache extends Cache { * @return array */ public function get($file) { - if ($file == '') { - $data = \OCP\Share::getItemsSharedWith('file', \OC_Share_Backend_File::FORMAT_FILE_APP_ROOT); - $etag = \OCP\Config::getUserValue(\OCP\User::getUser(), 'files_sharing', 'etag'); - if (!isset($etag)) { - $etag = $this->storage->getETag(''); - \OCP\Config::setUserValue(\OCP\User::getUser(), 'files_sharing', 'etag', $etag); - } - $data['etag'] = $etag; - return $data; - } else if (is_string($file)) { + if (is_string($file)) { if ($cache = $this->getSourceCache($file)) { return $cache->get($this->files[$file]); } diff --git a/apps/files_sharing/lib/share/file.php b/apps/files_sharing/lib/share/file.php index b5196ab6fa..f9f3211bd8 100644 --- a/apps/files_sharing/lib/share/file.php +++ b/apps/files_sharing/lib/share/file.php @@ -109,22 +109,6 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { $files[] = $file; } return $files; - } else if ($format == self::FORMAT_FILE_APP_ROOT) { - $mtime = 0; - $size = 0; - foreach ($items as $item) { - if ($item['mtime'] > $mtime) { - $mtime = $item['mtime']; - } - $size += (int)$item['size']; - } - return array( - 'fileid' => -1, - 'name' => 'Shared', - 'mtime' => $mtime, - 'mimetype' => 'httpd/unix-directory', - 'size' => $size - ); } else if ($format == self::FORMAT_OPENDIR) { $files = array(); foreach ($items as $item) { From 72bbb9ca20498eba18d6b6a31ed1de2306f90faf Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 7 Apr 2014 19:00:03 +0200 Subject: [PATCH 05/34] allow to remove and change mount points --- lib/private/files/mount/manager.php | 7 +++++++ lib/private/files/mount/mount.php | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/lib/private/files/mount/manager.php b/lib/private/files/mount/manager.php index ff4a336f34..91460b7273 100644 --- a/lib/private/files/mount/manager.php +++ b/lib/private/files/mount/manager.php @@ -23,6 +23,13 @@ class Manager { $this->mounts[$mount->getMountPoint()] = $mount; } + /** + * @param string $mountPoint + */ + public function removeMount($mountPoint) { + unset($this->mounts[$mountPoint]); + } + /** * Find the mount for $path * diff --git a/lib/private/files/mount/mount.php b/lib/private/files/mount/mount.php index 0ce2f5975c..08d5ddf348 100644 --- a/lib/private/files/mount/mount.php +++ b/lib/private/files/mount/mount.php @@ -65,6 +65,13 @@ class Mount { return $this->mountPoint; } + /** + * @param string $mountPoint new mount point + */ + public function setMountPoint($mountPoint) { + $this->mountPoint = $mountPoint; + } + /** * create the storage that is mounted * From a02fb3722bae6655ffdd5b0e6c522331612c255b Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 4 Apr 2014 18:32:49 +0200 Subject: [PATCH 06/34] user should be able to rename/delete shared files if the owner allowed it --- apps/files_sharing/lib/cache.php | 9 ++ apps/files_sharing/lib/sharedstorage.php | 102 ++++++++++++++++++++++- lib/private/files/view.php | 29 +++++-- 3 files changed, 129 insertions(+), 11 deletions(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index becd436f79..4017b7ad64 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -89,6 +89,12 @@ class Shared_Cache extends Cache { return $cache->get($this->files[$file]); } } else { + // if we are at the root of the mount point we want to return the + // cache information for the source item + if (!is_int($file) || $file === 0) { + $file = $this->storage->getSourceId(); + $mountPoint = $this->storage->getMountPoint(); + } $query = \OC_DB::prepare( 'SELECT `fileid`, `storage`, `path`, `parent`, `name`, `mimetype`, `mimepart`,' . ' `size`, `mtime`, `encrypted`, `unencrypted_size`' @@ -110,6 +116,9 @@ class Shared_Cache extends Cache { } else { $data['size'] = (int)$data['size']; } + if (isset($mountPoint)) { + $data['path'] = 'files/' . $mountPoint; + } return $data; } return false; diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 25a05a0d1f..d09a4a2256 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -30,17 +30,33 @@ class Shared extends \OC\Files\Storage\Common { private $mountPoint; // mount point relative to data/user/files private $type; // can be "file" or "folder" + private $shareId; // share Id to identify the share in the database + private $fileSource; // file cache ID of the shared item private $files = array(); public function __construct($arguments) { $this->mountPoint = $arguments['shareTarget']; $this->type = $arguments['shareType']; + $this->shareId = $arguments['shareId']; + $this->fileSource = $arguments['fileSource']; } + /** + * @breif get id of the mount point + * @return string + */ public function getId() { return 'shared::' . $this->mountPoint; } + /** + * @breif get file cache of the shared item source + * @return string + */ + public function getSourceId() { + return $this->fileSource; + } + /** * @brief Get the source file path, permissions, and owner for a shared file * @param string Shared target file path @@ -289,11 +305,89 @@ class Shared extends \OC\Files\Storage\Common { return false; } + /** + * @brief Format a path to be relative to the /user/files/ directory + * @param string $path the absolute path + * @return string e.g. turns '/admin/files/test.txt' into '/test.txt' + */ + private static function stripUserFilesPath($path) { + $trimmed = ltrim($path, '/'); + $split = explode('/', $trimmed); + + // it is not a file relative to data/user/files + if (count($split) < 3 || $split[1] !== 'files') { + \OCP\Util::writeLog('file sharing', + 'Can not strip userid and "files/" from path: ' . $path, + \OCP\Util::DEBUG); + return false; + } + + // skip 'user' and 'files' + $sliced = array_slice($split, 2); + $relPath = implode('/', $sliced); + + return '/' . $relPath; + } + + /** + * @brief rename a shared foder/file + * @param string $sourcePath + * @param string $targetPath + * @return bool + */ + private function renameMountPoint($sourcePath, $targetPath) { + + // it shoulbn't be possible to move a Shared storage into another one + list($targetStorage, ) = \OC\Files\Filesystem::resolvePath($targetPath); + if ($targetStorage instanceof \OC\Files\Storage\Shared) { + \OCP\Util::writeLog('file sharing', + 'It is not allowed to move one mount point into another one', + \OCP\Util::DEBUG); + return false; + } + + $relTargetPath = $this->stripUserFilesPath($targetPath); + + // rename mount point + $query = \OC_DB::prepare( + 'Update `*PREFIX*share` + SET `file_target` = ? + WHERE `id` = ?' + ); + + $result = $query->execute(array($relTargetPath, $this->shareId)); + + if ($result) { + // update the mount manager with the new paths + $mountManager = \OC\Files\Filesystem::getMountManager(); + $mount = $mountManager->find($sourcePath); + $mount->setMountPoint($targetPath . '/'); + $mountManager->addMount($mount); + $mountManager->removeMount($sourcePath . '/'); + + } else { + \OCP\Util::writeLog('file sharing', + 'Could not rename mount point for shared folder "' . $sourcePath . '" to "' . $targetPath . '"', + \OCP\Util::ERROR); + } + + return $result; + } + + public function rename($path1, $path2) { + + $sourceMountPoint = \OC\Files\Filesystem::getMountPoint($path1); + + // if we renamed the mount point we need to adjust the file_target in the + // database + if (strlen($sourceMountPoint) >= strlen($path1)) { + return $this->renameMountPoint($path1, $path2); + } + // Renaming/moving is only allowed within shared folders - $pos1 = strpos($path1, '/', 1); - $pos2 = strpos($path2, '/', 1); - if ($pos1 !== false && $pos2 !== false && ($oldSource = $this->getSourcePath($path1))) { + $oldSource = $this->getSourcePath($path1); + if ($oldSource) { $newSource = $this->getSourcePath(dirname($path2)) . '/' . basename($path2); // Within the same folder, we only need UPDATE permissions if (dirname($path1) == dirname($path2) and $this->isUpdatable($path1)) { @@ -405,6 +499,8 @@ class Shared extends \OC\Files\Storage\Common { array( 'shareTarget' => $share['file_target'], 'shareType' => $share['item_type'], + 'shareId' => $share['id'], + 'fileSource' => $share['file_source'], ), $options['user_dir'] . '/' . $share['file_target']); } diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 519ed250b1..6d630f978b 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -404,11 +404,21 @@ class View { if ($run) { $mp1 = $this->getMountPoint($path1 . $postFix1); $mp2 = $this->getMountPoint($path2 . $postFix2); + list($storage1, $internalPath1) = Filesystem::resolvePath($absolutePath1 . $postFix1); + list(, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2); + // if source and target are on the same storage we can call the rename operation from the + // storage. If it is a "Shared" file/folder we call always the rename operation of the + // shared storage to handle mount point renaming, etc correctly if ($mp1 == $mp2) { - list($storage, $internalPath1) = Filesystem::resolvePath($absolutePath1 . $postFix1); - list(, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2); - if ($storage) { - $result = $storage->rename($internalPath1, $internalPath2); + if ($storage1) { + $result = $storage1->rename($internalPath1, $internalPath2); + \OC_FileProxy::runPostProxies('rename', $absolutePath1, $absolutePath2); + } else { + $result = false; + } + } elseif ($storage1 instanceof \OC\Files\Storage\Shared) { + if ($storage1) { + $result = $storage1->rename($absolutePath1, $absolutePath2); \OC_FileProxy::runPostProxies('rename', $absolutePath1, $absolutePath2); } else { $result = false; @@ -417,7 +427,6 @@ class View { if ($this->is_dir($path1)) { $result = $this->copy($path1, $path2); if ($result === true) { - list($storage1, $internalPath1) = Filesystem::resolvePath($absolutePath1 . $postFix1); $result = $storage1->unlink($internalPath1); } } else { @@ -431,7 +440,6 @@ class View { fclose($target); if ($result !== false) { - list($storage1, $internalPath1) = Filesystem::resolvePath($absolutePath1 . $postFix1); $storage1->unlink($internalPath1); } } @@ -972,8 +980,13 @@ class View { $permissions = $subStorage->getPermissions($rootEntry['path']); $subPermissionsCache->set($rootEntry['fileid'], $user, $permissions); } - // do not allow renaming/deleting the mount point - $rootEntry['permissions'] = $permissions & (\OCP\PERMISSION_ALL - (\OCP\PERMISSION_UPDATE | \OCP\PERMISSION_DELETE)); + // do not allow renaming/deleting the mount point if they are not shared files/folders + // for shared files/folders we use the permissions given by the owner + if ($subStorage instanceof \OC\Files\Storage\Shared) { + $rootEntry['permissions'] = $permissions; + } else { + $rootEntry['permissions'] = $permissions & (\OCP\PERMISSION_ALL - (\OCP\PERMISSION_UPDATE | \OCP\PERMISSION_DELETE)); + } //remove any existing entry with the same name foreach ($files as $i => $file) { From 6d87dacad4e7b0d5ef92b92d892a457bdcbd207e Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 8 Apr 2014 11:32:39 +0200 Subject: [PATCH 07/34] fix getMimeType call, we always need to check the source path --- apps/files_sharing/lib/sharedstorage.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index d09a4a2256..3fa2540b60 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -452,9 +452,6 @@ class Shared extends \OC\Files\Storage\Common { } public function getMimeType($path) { - if ($path == '' || $path == '/') { - return 'httpd/unix-directory'; - } if ($source = $this->getSourcePath($path)) { list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); return $storage->getMimeType($internalPath); From c4e0fb75a4e8c9cb133ce79b6a737b2ba7b161df Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 8 Apr 2014 14:42:15 +0200 Subject: [PATCH 08/34] add api to get shares from a specific user --- lib/private/share/share.php | 16 ++++++++++++++++ lib/public/share.php | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/lib/private/share/share.php b/lib/private/share/share.php index 3751b035bd..ff56b9a48f 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -252,6 +252,22 @@ class Share extends \OC\Share\Constants { $parameters, $limit, $includeCollections); } + /** + * Get the items of item type shared with a user + * @param string Item type + * @param sting user id for which user we want the shares + * @param int Format (optional) Format type must be defined by the backend + * @param mixed Parameters (optional) + * @param int Number of items to return (optional) Returns all by default + * @param bool include collections (optional) + * @return Return depends on format + */ + public static function getItemsSharedWithUser($itemType, $user, $format = self::FORMAT_NONE, + $parameters = null, $limit = -1, $includeCollections = false) { + return self::getItems($itemType, null, self::$shareTypeUserAndGroups, $user, null, $format, + $parameters, $limit, $includeCollections); + } + /** * Get the item of item type shared with the current user * @param string $itemType diff --git a/lib/public/share.php b/lib/public/share.php index c694314ad0..230a517b5e 100644 --- a/lib/public/share.php +++ b/lib/public/share.php @@ -90,6 +90,22 @@ class Share extends \OC\Share\Constants { return \OC\Share\Share::getItemsSharedWith($itemType, $format, $parameters, $limit, $includeCollections); } + /** + * Get the items of item type shared with a user + * @param string Item type + * @param sting user id for which user we want the shares + * @param int Format (optional) Format type must be defined by the backend + * @param mixed Parameters (optional) + * @param int Number of items to return (optional) Returns all by default + * @param bool include collections (optional) + * @return Return depends on format + */ + public static function getItemsSharedWithUser($itemType, $user, $format = self::FORMAT_NONE, + $parameters = null, $limit = -1, $includeCollections = false) { + + return \OC\Share\Share::getItemsSharedWithUser($itemType, $user, $format, $parameters, $limit, $includeCollections); + } + /** * Get the item of item type shared with the current user * @param string $itemType From 6b19482f3e1d97aab63c6c870c53a27d5e5a8524 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 8 Apr 2014 12:29:47 +0200 Subject: [PATCH 09/34] generate unique target name --- apps/files_sharing/lib/share/file.php | 44 ++++++++++++++++++--------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/apps/files_sharing/lib/share/file.php b/apps/files_sharing/lib/share/file.php index f9f3211bd8..c628b11589 100644 --- a/apps/files_sharing/lib/share/file.php +++ b/apps/files_sharing/lib/share/file.php @@ -28,6 +28,7 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { const FORMAT_OPENDIR = 3; const FORMAT_GET_ALL = 4; const FORMAT_PERMISSIONS = 5; + const FORMAT_TARGET_NAMES = 6; private $path; @@ -50,24 +51,31 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { return false; } + /** + * @brief create unique target + * @param string $filePath + * @param string $shareWith + * @param string $exclude + * @return string + */ public function generateTarget($filePath, $shareWith, $exclude = null) { $target = '/'.basename($filePath); - if (isset($exclude)) { - if ($pos = strrpos($target, '.')) { - $name = substr($target, 0, $pos); - $ext = substr($target, $pos); - } else { - $name = $target; - $ext = ''; - } - $i = 2; - $append = ''; - while (in_array($name.$append.$ext, $exclude)) { - $append = ' ('.$i.')'; - $i++; - } - $target = $name.$append.$ext; + \OC\Files\Filesystem::initMountPoints($shareWith); + $view = new \OC\Files\View('/' . $shareWith . '/files'); + $excludeList = \OCP\Share::getItemsSharedWithUser('file', $shareWith, self::FORMAT_TARGET_NAMES); + if (is_array($exclude)) { + $excludeList = array_merge($excludeList, $exclude); } + + $pathinfo = pathinfo($target); + $ext = (isset($pathinfo['extension'])) ? '.'.$pathinfo['extension'] : ''; + $name = $pathinfo['filename']; + $i = 2; + while ($view->file_exists($target) || in_array($target, $excludeList)) { + $target = '/' . $name . ' ('.$i.')' . $ext; + $i++; + } + return $target; } @@ -127,6 +135,12 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { $filePermissions[$item['file_source']] = $item['permissions']; } return $filePermissions; + } else if ($format === self::FORMAT_TARGET_NAMES) { + $targets = array(); + foreach ($items as $item) { + $targets[] = $item['file_target']; + } + return $targets; } return array(); } From 496743523729b69e311c170a810182f95568fb7d Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 8 Apr 2014 16:37:34 +0200 Subject: [PATCH 10/34] show "shared by ..." for share mount point --- apps/files_sharing/lib/cache.php | 4 +++- apps/files_sharing/lib/sharedstorage.php | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index 4017b7ad64..250c1e872e 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -86,7 +86,9 @@ class Shared_Cache extends Cache { public function get($file) { if (is_string($file)) { if ($cache = $this->getSourceCache($file)) { - return $cache->get($this->files[$file]); + $data = $cache->get($this->files[$file]); + $data['displayname_owner'] = \OC_User::getDisplayName($this->storage->getSharedFrom()); + return $data; } } else { // if we are at the root of the mount point we want to return the diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 3fa2540b60..f38f29635a 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -32,6 +32,7 @@ class Shared extends \OC\Files\Storage\Common { private $type; // can be "file" or "folder" private $shareId; // share Id to identify the share in the database private $fileSource; // file cache ID of the shared item + private $sharedFrom; // the user who shared the file private $files = array(); public function __construct($arguments) { @@ -39,6 +40,7 @@ class Shared extends \OC\Files\Storage\Common { $this->type = $arguments['shareType']; $this->shareId = $arguments['shareId']; $this->fileSource = $arguments['fileSource']; + $this->sharedFrom = $arguments['sharedFrom']; } /** @@ -498,6 +500,7 @@ class Shared extends \OC\Files\Storage\Common { 'shareType' => $share['item_type'], 'shareId' => $share['id'], 'fileSource' => $share['file_source'], + 'sharedFrom' => $share['uid_owner'], ), $options['user_dir'] . '/' . $share['file_target']); } @@ -512,6 +515,14 @@ class Shared extends \OC\Files\Storage\Common { return ltrim($this->mountPoint, '/'); } + /** + * @brief get the user who shared the file + * @return string + */ + public function getSharedFrom() { + return $this->sharedFrom; + } + /** * @brief return share type, can be "file" or "folder" * @return string From 83d68107253834aa5322198295c827d94e951e90 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 8 Apr 2014 16:56:10 +0200 Subject: [PATCH 11/34] don't overwrite shared folder icon --- apps/files/lib/helper.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/files/lib/helper.php b/apps/files/lib/helper.php index 2e3741cbdc..88a5ffcfb6 100644 --- a/apps/files/lib/helper.php +++ b/apps/files/lib/helper.php @@ -37,8 +37,7 @@ class Helper $sid = explode(':', $sid); if ($sid[0] === 'shared') { $icon = \OC_Helper::mimetypeIcon('dir-shared'); - } - if ($sid[0] !== 'local' and $sid[0] !== 'home') { + } elseif ($sid[0] !== 'local' and $sid[0] !== 'home') { $icon = \OC_Helper::mimetypeIcon('dir-external'); } } From 27c5a978f91e7aa447a2acca040fd173baba53b9 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 8 Apr 2014 17:17:48 +0200 Subject: [PATCH 12/34] we no longer need to handle the Shared folder different from any other folder --- apps/files/js/file-upload.js | 4 +- apps/files/js/fileactions.js | 8 +-- apps/files/js/filelist.js | 2 +- apps/files/js/files.js | 6 +- apps/files/lib/app.php | 11 +-- apps/files/tests/ajax_rename.php | 82 ---------------------- apps/files/tests/js/filesSpec.js | 35 --------- lib/private/connector/sabre/directory.php | 12 ---- lib/private/connector/sabre/file.php | 11 --- lib/private/connector/sabre/objecttree.php | 3 - 10 files changed, 8 insertions(+), 166 deletions(-) diff --git a/apps/files/js/file-upload.js b/apps/files/js/file-upload.js index 3879aa6588..03ebdccb32 100644 --- a/apps/files/js/file-upload.js +++ b/apps/files/js/file-upload.js @@ -235,7 +235,7 @@ OC.Upload = { var file = data.files[0]; try { // FIXME: not so elegant... need to refactor that method to return a value - Files.isFileNameValid(file.name, FileList.getCurrentDirectory()); + Files.isFileNameValid(file.name); } catch (errorMessage) { data.textStatus = 'invalidcharacters'; @@ -555,8 +555,6 @@ OC.Upload = { throw t('files', 'URL cannot be empty'); } else if (type !== 'web' && !Files.isFileNameValid(filename)) { // Files.isFileNameValid(filename) throws an exception itself - } else if (FileList.getCurrentDirectory() === '/' && filename.toLowerCase() === 'shared') { - throw t('files', 'In the home folder \'Shared\' is a reserved filename'); } else if (FileList.inList(filename)) { throw t('files', '{new_name} already exists', {new_name: filename}); } else { diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 631aebea95..ecdfa72a47 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -118,10 +118,6 @@ var FileActions = { }; var addAction = function (name, action, displayName) { - // NOTE: Temporary fix to prevent rename action in root of Shared directory - if (name === 'Rename' && $('#dir').val() === '/Shared') { - return true; - } if ((name === 'Download' || action !== defaultAction) && name !== 'Delete') { @@ -160,7 +156,7 @@ var FileActions = { addAction(name, ah, displayName); } }); - if(actions.Share && !($('#dir').val() === '/' && file === 'Shared')){ + if(actions.Share){ displayName = t('files', 'Share'); addAction('Share', actions.Share, displayName); } @@ -223,7 +219,7 @@ $(document).ready(function () { $('#fileList tr').each(function () { FileActions.display($(this).children('td.filename')); }); - + $('#fileList').trigger(jQuery.Event("fileActionsReady")); }); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 9c749bb8f3..343da21741 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -580,7 +580,7 @@ window.FileList = { var filename = input.val(); if (filename !== oldname) { // Files.isFileNameValid(filename) throws an exception itself - Files.isFileNameValid(filename, FileList.getCurrentDirectory()); + Files.isFileNameValid(filename); if (FileList.inList(filename)) { throw t('files', '{new_name} already exists', {new_name: filename}); } diff --git a/apps/files/js/files.js b/apps/files/js/files.js index 9f38263bef..5e669a796a 100644 --- a/apps/files/js/files.js +++ b/apps/files/js/files.js @@ -87,11 +87,9 @@ var Files = { * Throws a string exception with an error message if * the file name is not valid */ - isFileNameValid: function (name, root) { + isFileNameValid: function (name) { var trimmedName = name.trim(); - if (trimmedName === '.' - || trimmedName === '..' - || (root === '/' && trimmedName.toLowerCase() === 'shared')) + if (trimmedName === '.' || trimmedName === '..') { throw t('files', '"{name}" is an invalid file name.', {name: name}); } else if (trimmedName.length === 0) { diff --git a/apps/files/lib/app.php b/apps/files/lib/app.php index adfca66957..ed4aa32c66 100644 --- a/apps/files/lib/app.php +++ b/apps/files/lib/app.php @@ -54,13 +54,8 @@ class App { 'data' => NULL ); - // rename to "/Shared" is denied - if( $dir === '/' and $newname === 'Shared' ) { - $result['data'] = array( - 'message' => $this->l10n->t("Invalid folder name. Usage of 'Shared' is reserved.") - ); // rename to non-existing folder is denied - } else if (!$this->view->file_exists($dir)) { + if (!$this->view->file_exists($dir)) { $result['data'] = array('message' => (string)$this->l10n->t( 'The target folder has been moved or deleted.', array($dir)), @@ -68,7 +63,7 @@ class App { ); // rename to existing file is denied } else if ($this->view->file_exists($dir . '/' . $newname)) { - + $result['data'] = array( 'message' => $this->l10n->t( "The name %s is already used in the folder %s. Please choose a different name.", @@ -77,8 +72,6 @@ class App { } else if ( // rename to "." is denied $newname !== '.' and - // rename of "/Shared" is denied - !($dir === '/' and $oldname === 'Shared') and // THEN try to rename $this->view->rename($dir . '/' . $oldname, $dir . '/' . $newname) ) { diff --git a/apps/files/tests/ajax_rename.php b/apps/files/tests/ajax_rename.php index cb62d22a7e..74ca1e4495 100644 --- a/apps/files/tests/ajax_rename.php +++ b/apps/files/tests/ajax_rename.php @@ -55,88 +55,6 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase { \OC\Files\Filesystem::tearDown(); } - /** - * @brief test rename of file/folder named "Shared" - */ - function testRenameSharedFolder() { - $dir = '/'; - $oldname = 'Shared'; - $newname = 'new_name'; - - $this->viewMock->expects($this->at(0)) - ->method('file_exists') - ->with('/') - ->will($this->returnValue(true)); - - $result = $this->files->rename($dir, $oldname, $newname); - $expected = array( - 'success' => false, - 'data' => array('message' => '%s could not be renamed') - ); - - $this->assertEquals($expected, $result); - } - - /** - * @brief test rename of file/folder named "Shared" - */ - function testRenameSharedFolderInSubdirectory() { - $dir = '/test'; - $oldname = 'Shared'; - $newname = 'new_name'; - - $this->viewMock->expects($this->at(0)) - ->method('file_exists') - ->with('/test') - ->will($this->returnValue(true)); - - $this->viewMock->expects($this->any()) - ->method('getFileInfo') - ->will($this->returnValue(new \OC\Files\FileInfo( - '/test', - null, - '/test', - array( - 'fileid' => 123, - 'type' => 'dir', - 'mimetype' => 'httpd/unix-directory', - 'mtime' => 0, - 'permissions' => 31, - 'size' => 18, - 'etag' => 'abcdef', - 'directory' => '/', - 'name' => 'new_name', - )))); - - $result = $this->files->rename($dir, $oldname, $newname); - - $this->assertTrue($result['success']); - $this->assertEquals(123, $result['data']['id']); - $this->assertEquals('new_name', $result['data']['name']); - $this->assertEquals(18, $result['data']['size']); - $this->assertEquals('httpd/unix-directory', $result['data']['mimetype']); - $icon = \OC_Helper::mimetypeIcon('dir'); - $icon = substr($icon, 0, -3) . 'svg'; - $this->assertEquals($icon, $result['data']['icon']); - } - - /** - * @brief test rename of file/folder to "Shared" - */ - function testRenameFolderToShared() { - $dir = '/'; - $oldname = 'oldname'; - $newname = 'Shared'; - - $result = $this->files->rename($dir, $oldname, $newname); - $expected = array( - 'success' => false, - 'data' => array('message' => "Invalid folder name. Usage of 'Shared' is reserved.") - ); - - $this->assertEquals($expected, $result); - } - /** * @brief test rename of file/folder */ diff --git a/apps/files/tests/js/filesSpec.js b/apps/files/tests/js/filesSpec.js index 95bf87e03e..018c8ef0f3 100644 --- a/apps/files/tests/js/filesSpec.js +++ b/apps/files/tests/js/filesSpec.js @@ -48,41 +48,6 @@ describe('Files tests', function() { expect(error).toEqual(false); } }); - it('Validates correct file names do not create Shared folder in root', function() { - // create shared file in subfolder - var error = false; - try { - expect(Files.isFileNameValid('shared', '/foo')).toEqual(true); - expect(Files.isFileNameValid('Shared', '/foo')).toEqual(true); - } - catch (e) { - error = e; - } - expect(error).toEqual(false); - - // create shared file in root - var threwException = false; - try { - Files.isFileNameValid('Shared', '/'); - console.error('Invalid file name not detected'); - } - catch (e) { - threwException = true; - } - expect(threwException).toEqual(true); - - // create shared file in root - var threwException = false; - try { - Files.isFileNameValid('shared', '/'); - console.error('Invalid file name not detected'); - } - catch (e) { - threwException = true; - } - expect(threwException).toEqual(true); - - }); it('Detects invalid file names', function() { var fileNames = [ '', diff --git a/lib/private/connector/sabre/directory.php b/lib/private/connector/sabre/directory.php index 70f45aa1e7..5d386a772a 100644 --- a/lib/private/connector/sabre/directory.php +++ b/lib/private/connector/sabre/directory.php @@ -50,10 +50,6 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa */ public function createFile($name, $data = null) { - if (strtolower($name) === 'shared' && empty($this->path)) { - throw new \Sabre_DAV_Exception_Forbidden(); - } - // for chunked upload also updating a existing file is a "createFile" // because we create all the chunks before reasamble them to the existing file. if (isset($_SERVER['HTTP_OC_CHUNKED'])) { @@ -86,10 +82,6 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa */ public function createDirectory($name) { - if (strtolower($name) === 'shared' && empty($this->path)) { - throw new \Sabre_DAV_Exception_Forbidden(); - } - if (!\OC\Files\Filesystem::isCreatable($this->path)) { throw new \Sabre_DAV_Exception_Forbidden(); } @@ -196,10 +188,6 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa */ public function delete() { - if ($this->path === 'Shared') { - throw new \Sabre_DAV_Exception_Forbidden(); - } - if (!\OC\Files\Filesystem::isDeletable($this->path)) { throw new \Sabre_DAV_Exception_Forbidden(); } diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index 750d646a8f..433b4d805c 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -71,13 +71,6 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D // mark file as partial while uploading (ignored by the scanner) $partpath = $this->path . '.ocTransferId' . rand() . '.part'; - // if file is located in /Shared we write the part file to the users - // root folder because we can't create new files in /shared - // we extend the name with a random number to avoid overwriting a existing file - if (dirname($partpath) === 'Shared') { - $partpath = pathinfo($partpath, PATHINFO_FILENAME) . rand() . '.part'; - } - try { $putOkay = $fs->file_put_contents($partpath, $data); if ($putOkay === false) { @@ -149,10 +142,6 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D public function delete() { $fs = $this->getFS(); - if ($this->path === 'Shared') { - throw new \Sabre_DAV_Exception_Forbidden(); - } - if (!$fs->isDeletable($this->path)) { throw new \Sabre_DAV_Exception_Forbidden(); } diff --git a/lib/private/connector/sabre/objecttree.php b/lib/private/connector/sabre/objecttree.php index accf020daa..d2fa425b22 100644 --- a/lib/private/connector/sabre/objecttree.php +++ b/lib/private/connector/sabre/objecttree.php @@ -94,9 +94,6 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { } if ($sourceDir !== $destinationDir) { // for a full move we need update privileges on sourcePath and sourceDir as well as destinationDir - if (ltrim($destinationDir, '/') === '' && strtolower($sourceNode->getName()) === 'shared') { - throw new \Sabre_DAV_Exception_Forbidden(); - } if (!$fs->isUpdatable($sourceDir)) { throw new \Sabre_DAV_Exception_Forbidden(); } From ed981294f11bd59733e0d121cbf737e16275b666 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 8 Apr 2014 19:57:07 +0200 Subject: [PATCH 13/34] fix share api tests --- apps/files_sharing/lib/api.php | 12 ---- apps/files_sharing/lib/cache.php | 21 ++++--- apps/files_sharing/lib/share/file.php | 2 +- apps/files_sharing/lib/sharedstorage.php | 39 +++---------- apps/files_sharing/lib/watcher.php | 2 +- apps/files_sharing/tests/api.php | 70 +++++------------------- apps/files_sharing/tests/base.php | 12 ++-- apps/files_sharing/tests/cache.php | 60 +++++++++----------- apps/files_sharing/tests/permissions.php | 15 +++-- apps/files_sharing/tests/watcher.php | 33 ++++------- lib/private/share/share.php | 6 +- 11 files changed, 91 insertions(+), 181 deletions(-) diff --git a/apps/files_sharing/lib/api.php b/apps/files_sharing/lib/api.php index de3c1cd263..438d3cc4ba 100644 --- a/apps/files_sharing/lib/api.php +++ b/apps/files_sharing/lib/api.php @@ -184,7 +184,6 @@ class Api { $receivedFrom = \OCP\Share::getItemSharedWithBySource($itemType, $file['fileid']); reset($share); $key = key($share); - $share[$key]['path'] = self::correctPath($share[$key]['path'], $path); if ($receivedFrom) { $share[$key]['received_from'] = $receivedFrom['uid_owner']; $share[$key]['received_from_displayname'] = \OCP\User::getDisplayName($receivedFrom['uid_owner']); @@ -531,15 +530,4 @@ class Api { } - /** - * @brief make sure that the path has the correct root - * - * @param string $path path returned from the share API - * @param string $folder current root folder - * @return string the correct path - */ - protected static function correctPath($path, $folder) { - return \OC_Filesystem::normalizePath('/' . $folder . '/' . basename($path)); - } - } diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index 250c1e872e..e91c15cc62 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -47,7 +47,7 @@ class Shared_Cache extends Cache { * @return \OC\Files\Cache\Cache */ private function getSourceCache($target) { - if ($target === false) { + if ($target === false || $target === $this->storage->getMountPoint()) { $target = ''; } $source = \OC_Share_Backend_File::getSource($target, $this->storage->getMountPoint(), $this->storage->getShareType()); @@ -86,8 +86,11 @@ class Shared_Cache extends Cache { public function get($file) { if (is_string($file)) { if ($cache = $this->getSourceCache($file)) { + $path = 'files/' . $this->storage->getMountPoint(); + $path .= ($file !== '') ? '/' . $file : ''; $data = $cache->get($this->files[$file]); $data['displayname_owner'] = \OC_User::getDisplayName($this->storage->getSharedFrom()); + $data['path'] = $path; return $data; } } else { @@ -99,7 +102,7 @@ class Shared_Cache extends Cache { } $query = \OC_DB::prepare( 'SELECT `fileid`, `storage`, `path`, `parent`, `name`, `mimetype`, `mimepart`,' - . ' `size`, `mtime`, `encrypted`, `unencrypted_size`' + . ' `size`, `mtime`, `encrypted`, `unencrypted_size`, `storage_mtime`' . ' FROM `*PREFIX*filecache` WHERE `fileid` = ?'); $result = $query->execute(array($file)); $data = $result->fetchRow(); @@ -138,12 +141,15 @@ class Shared_Cache extends Cache { $folder = ''; } + $dir = 'files/' . $this->storage->getMountPoint(); + $dir .= ($folder !== '') ? '/' . $folder : ''; + $cache = $this->getSourceCache($folder); if ($cache) { $parent = $this->storage->getFile($folder); $sourceFolderContent = $cache->getFolderContents($this->files[$folder]); foreach ($sourceFolderContent as $key => $c) { - $sourceFolderContent[$key]['usersPath'] = 'files/' . $folder . '/' . $c['name']; + $sourceFolderContent[$key]['path'] = $dir . '/' . $c['name']; $sourceFolderContent[$key]['uid_owner'] = $parent['uid_owner']; $sourceFolderContent[$key]['displayname_owner'] = $parent['uid_owner']; } @@ -178,7 +184,11 @@ class Shared_Cache extends Cache { * @return int */ public function getId($file) { - if ($cache = $this->getSourceCache($file)) { + if ($file === false) { + return $this->storage->getSourceId(); + } + $cache = $this->getSourceCache($file); + if ($cache) { return $cache->getId($this->files[$file]); } return -1; @@ -292,9 +302,6 @@ class Shared_Cache extends Cache { if ($file['mimetype'] === 'httpd/unix-directory') { $exploreDirs[] = ltrim($dir . '/' . $file['name'], '/'); } else if (($mimepart && $file['mimepart'] === $mimepart) || ($mimetype && $file['mimetype'] === $mimetype)) { - // usersPath not reliable - //$file['path'] = $file['usersPath']; - $file['path'] = ltrim($dir . '/' . $file['name'], '/'); $result[] = $file; } } diff --git a/apps/files_sharing/lib/share/file.php b/apps/files_sharing/lib/share/file.php index c628b11589..e1d0b49706 100644 --- a/apps/files_sharing/lib/share/file.php +++ b/apps/files_sharing/lib/share/file.php @@ -87,7 +87,7 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { 'path' => $items[key($items)]['path'], 'storage' => $items[key($items)]['storage'], 'permissions' => $items[key($items)]['permissions'], - 'uid_owner' => $items[key($items)]['uid_owner'] + 'uid_owner' => $items[key($items)]['uid_owner'], ); } else if ($format == self::FORMAT_GET_FOLDER_CONTENTS) { $files = array(); diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index f38f29635a..565c3e7af8 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -138,15 +138,9 @@ class Shared extends \OC\Files\Storage\Common { } public function opendir($path) { - if ($path == '' || $path == '/') { - $files = \OCP\Share::getItemsSharedWith('file', \OC_Share_Backend_Folder::FORMAT_OPENDIR); - \OC\Files\Stream\Dir::register($this->mountPoint, $files); - return opendir('fakedir://' . $this->mountPoint); - } else if ($source = $this->getSourcePath($path)) { - list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); - return $storage->opendir($internalPath); - } - return false; + $source = $this->getSourcePath($path); + list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); + return $storage->opendir($internalPath); } public function is_dir($path) { @@ -242,25 +236,9 @@ class Shared extends \OC\Files\Storage\Common { } public function filemtime($path) { - if ($path == '' || $path == '/') { - $mtime = 0; - $dh = $this->opendir($path); - if (is_resource($dh)) { - while (($filename = readdir($dh)) !== false) { - $tempmtime = $this->filemtime($filename); - if ($tempmtime > $mtime) { - $mtime = $tempmtime; - } - } - } - return $mtime; - } else { - $source = $this->getSourcePath($path); - if ($source) { - list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); - return $storage->filemtime($internalPath); - } - } + $source = $this->getSourcePath($path); + list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); + return $storage->filemtime($internalPath); } public function file_get_contents($path) { @@ -285,7 +263,7 @@ class Shared extends \OC\Files\Storage\Common { return false; } $info = array( - 'target' => $this->mountPoint . $path, + 'target' => $this->mountPoint . '/' . $path, 'source' => $source, ); \OCP\Util::emitHook('\OC\Files\Storage\Shared', 'file_put_contents', $info); @@ -532,9 +510,6 @@ class Shared extends \OC\Files\Storage\Common { } public function hasUpdated($path, $time) { - if ($path == '') { - return false; - } return $this->filemtime($path) > $time; } diff --git a/apps/files_sharing/lib/watcher.php b/apps/files_sharing/lib/watcher.php index 285b1a58c6..11d3ce1cab 100644 --- a/apps/files_sharing/lib/watcher.php +++ b/apps/files_sharing/lib/watcher.php @@ -32,7 +32,7 @@ class Shared_Watcher extends Watcher { * @param string $path */ public function checkUpdate($path) { - if ($path != '' && parent::checkUpdate($path) === true) { + if (parent::checkUpdate($path) === true) { // since checkUpdate() has already updated the size of the subdirs, // only apply the update to the owner's parent dirs diff --git a/apps/files_sharing/tests/api.php b/apps/files_sharing/tests/api.php index c7a848315a..6354d1099b 100644 --- a/apps/files_sharing/tests/api.php +++ b/apps/files_sharing/tests/api.php @@ -324,10 +324,10 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { $this->assertTrue(is_string($result)); $testValues=array( - array('query' => 'Shared/' . $this->folder, - 'expectedResult' => '/Shared' . $this->folder . $this->filename), - array('query' => 'Shared/' . $this->folder . $this->subfolder, - 'expectedResult' => '/Shared' . $this->folder . $this->subfolder . $this->filename), + array('query' => $this->folder, + 'expectedResult' => $this->folder . $this->filename), + array('query' => $this->folder . $this->subfolder, + 'expectedResult' => $this->folder . $this->subfolder . $this->filename), ); foreach ($testValues as $value) { @@ -382,7 +382,7 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { // share was successful? $this->assertTrue(is_string($result)); - $_GET['path'] = '/Shared'; + $_GET['path'] = '/'; $_GET['subfiles'] = 'true'; $result = Share\Api::getAllShares(array()); @@ -395,7 +395,7 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { // we should get exactly one result $this->assertEquals(1, count($data)); - $expectedPath = '/Shared' . $this->subfolder; + $expectedPath = $this->subfolder; $this->assertEquals($expectedPath, $data[0]['path']); // cleanup @@ -444,7 +444,7 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { $this->assertTrue(is_string($result)); - $_GET['path'] = '/Shared'; + $_GET['path'] = '/'; $_GET['subfiles'] = 'true'; $result = Share\Api::getAllShares(array()); @@ -457,7 +457,7 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { // we should get exactly one result $this->assertEquals(1, count($data)); - $expectedPath = '/Shared' . $this->subsubfolder; + $expectedPath = $this->subsubfolder; $this->assertEquals($expectedPath, $data[0]['path']); @@ -512,8 +512,8 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { $this->assertTrue(is_string($result)); - // ask for shared/subfolder - $expectedPath1 = '/Shared' . $this->subfolder; + // ask for subfolder + $expectedPath1 = $this->subfolder; $_GET['path'] = $expectedPath1; $result1 = Share\Api::getAllShares(array()); @@ -524,8 +524,8 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { $data1 = $result1->getData(); $share1 = reset($data1); - // ask for shared/folder/subfolder - $expectedPath2 = '/Shared' . $this->folder . $this->subfolder; + // ask for folder/subfolder + $expectedPath2 = $this->folder . $this->subfolder; $_GET['path'] = $expectedPath2; $result2 = Share\Api::getAllShares(array()); @@ -595,7 +595,7 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { $this->assertTrue(is_string($result)); - $_GET['path'] = '/Shared'; + $_GET['path'] = '/'; $_GET['subfiles'] = 'true'; $result = Share\Api::getAllShares(array()); @@ -608,7 +608,7 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { // we should get exactly one result $this->assertEquals(1, count($data)); - $expectedPath = '/Shared' . $this->filename; + $expectedPath = $this->filename; $this->assertEquals($expectedPath, $data[0]['path']); @@ -868,46 +868,4 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { } - function testCorrectPath() { - $path = "/foo/bar/test.txt"; - $folder = "/correct/path"; - $expectedResult = "/correct/path/test.txt"; - - $shareApiDummy = new TestShareApi(); - - $this->assertSame($expectedResult, $shareApiDummy->correctPathTest($path, $folder)); - } - - /** - * @expectedException \Exception - */ - public function testShareNonExisting() { - \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER1); - - $id = PHP_INT_MAX - 1; - \OCP\Share::shareItem('file', $id, \OCP\Share::SHARE_TYPE_LINK, \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2, 31); - } - - /** - * @expectedException \Exception - */ - public function testShareNotOwner() { - \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2); - \OC\Files\Filesystem::file_put_contents('foo.txt', 'bar'); - $info = \OC\Files\Filesystem::getFileInfo('foo.txt'); - - \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER1); - - \OCP\Share::shareItem('file', $info->getId(), \OCP\Share::SHARE_TYPE_LINK, \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2, 31); - } - -} - -/** - * @brief dumnmy class to test protected methods - */ -class TestShareApi extends \OCA\Files\Share\Api { - public function correctPathTest($path, $folder) { - return self::correctPath($path, $folder); - } } diff --git a/apps/files_sharing/tests/base.php b/apps/files_sharing/tests/base.php index d44972d01f..495dca072c 100644 --- a/apps/files_sharing/tests/base.php +++ b/apps/files_sharing/tests/base.php @@ -102,22 +102,20 @@ abstract class Test_Files_Sharing_Base extends \PHPUnit_Framework_TestCase { * @param bool $password */ protected static function loginHelper($user, $create = false, $password = false) { - if ($create) { - \OC_User::createUser($user, $user); - } if ($password === false) { $password = $user; } + if ($create) { + \OC_User::createUser($user, $password); + } + \OC_Util::tearDownFS(); \OC_User::setUserId(''); \OC\Files\Filesystem::tearDown(); - \OC_Util::setupFS($user); \OC_User::setUserId($user); - - $params['uid'] = $user; - $params['password'] = $password; + \OC_Util::setupFS($user); } /** diff --git a/apps/files_sharing/tests/cache.php b/apps/files_sharing/tests/cache.php index 47969833ab..7a52f403d8 100644 --- a/apps/files_sharing/tests/cache.php +++ b/apps/files_sharing/tests/cache.php @@ -68,7 +68,7 @@ class Test_Files_Sharing_Cache extends Test_Files_Sharing_Base { // retrieve the shared storage $secondView = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2); - list($this->sharedStorage, $internalPath) = $secondView->resolvePath('files/Shared/shareddir'); + list($this->sharedStorage, $internalPath) = $secondView->resolvePath('files/shareddir'); $this->sharedCache = $this->sharedStorage->getCache(); } @@ -98,46 +98,46 @@ class Test_Files_Sharing_Cache extends Test_Files_Sharing_Base { function testSearchByMime() { $results = $this->sharedStorage->getCache()->searchByMime('text'); $check = array( - array( - 'name' => 'shared single file.txt', - 'path' => 'shared single file.txt' - ), array( 'name' => 'bar.txt', - 'path' => 'shareddir/bar.txt' + 'path' => 'files/shareddir/bar.txt' ), array( 'name' => 'another too.txt', - 'path' => 'shareddir/subdir/another too.txt' + 'path' => 'files/shareddir/subdir/another too.txt' ), array( 'name' => 'another.txt', - 'path' => 'shareddir/subdir/another.txt' + 'path' => 'files/shareddir/subdir/another.txt' ), ); $this->verifyFiles($check, $results); - $results2 = $this->sharedStorage->getCache()->searchByMime('text/plain'); - $this->verifyFiles($check, $results); } function testGetFolderContentsInRoot() { - $results = $this->user2View->getDirectoryContent('/Shared/'); + $results = $this->user2View->getDirectoryContent('/'); + // we should get the shared items "shareddir" and "shared single file.txt" + // additional root will always contain the example file "welcome.txt", + // so this will be part of the result $this->verifyFiles( array( + array( + 'name' => 'welcome.txt', + 'path' => 'files/welcome.txt', + 'mimetype' => 'text/plain', + ), array( 'name' => 'shareddir', - 'path' => '/shareddir', + 'path' => 'files/shareddir', 'mimetype' => 'httpd/unix-directory', - 'usersPath' => 'files/Shared/shareddir' ), array( 'name' => 'shared single file.txt', - 'path' => '/shared single file.txt', + 'path' => 'files/shared single file.txt', 'mimetype' => 'text/plain', - 'usersPath' => 'files/Shared/shared single file.txt' ), ), $results @@ -145,27 +145,24 @@ class Test_Files_Sharing_Cache extends Test_Files_Sharing_Base { } function testGetFolderContentsInSubdir() { - $results = $this->user2View->getDirectoryContent('/Shared/shareddir'); + $results = $this->user2View->getDirectoryContent('/shareddir'); $this->verifyFiles( array( array( 'name' => 'bar.txt', - 'path' => 'files/container/shareddir/bar.txt', + 'path' => 'files/shareddir/bar.txt', 'mimetype' => 'text/plain', - 'usersPath' => 'files/Shared/shareddir/bar.txt' ), array( 'name' => 'emptydir', - 'path' => 'files/container/shareddir/emptydir', + 'path' => 'files/shareddir/emptydir', 'mimetype' => 'httpd/unix-directory', - 'usersPath' => 'files/Shared/shareddir/emptydir' ), array( 'name' => 'subdir', - 'path' => 'files/container/shareddir/subdir', + 'path' => 'files/shareddir/subdir', 'mimetype' => 'httpd/unix-directory', - 'usersPath' => 'files/Shared/shareddir/subdir' ), ), $results @@ -182,27 +179,24 @@ class Test_Files_Sharing_Cache extends Test_Files_Sharing_Base { self::loginHelper(self::TEST_FILES_SHARING_API_USER3); $thirdView = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER3 . '/files'); - $results = $thirdView->getDirectoryContent('/Shared/subdir'); + $results = $thirdView->getDirectoryContent('/subdir'); $this->verifyFiles( array( array( 'name' => 'another too.txt', - 'path' => 'files/container/shareddir/subdir/another too.txt', + 'path' => 'files/subdir/another too.txt', 'mimetype' => 'text/plain', - 'usersPath' => 'files/Shared/subdir/another too.txt' ), array( 'name' => 'another.txt', - 'path' => 'files/container/shareddir/subdir/another.txt', + 'path' => 'files/subdir/another.txt', 'mimetype' => 'text/plain', - 'usersPath' => 'files/Shared/subdir/another.txt' ), array( 'name' => 'not a text file.xml', - 'path' => 'files/container/shareddir/subdir/not a text file.xml', + 'path' => 'files/subdir/not a text file.xml', 'mimetype' => 'application/xml', - 'usersPath' => 'files/Shared/subdir/not a text file.xml' ), ), $results @@ -254,8 +248,8 @@ class Test_Files_Sharing_Cache extends Test_Files_Sharing_Base { \OC_Util::tearDownFS(); self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - $this->assertTrue(\OC\Files\Filesystem::file_exists('/Shared/test.txt')); - list($sharedStorage) = \OC\Files\Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/Shared/test.txt'); + $this->assertTrue(\OC\Files\Filesystem::file_exists('/test.txt')); + list($sharedStorage) = \OC\Files\Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/test.txt'); /** * @var \OC\Files\Storage\Shared $sharedStorage */ @@ -275,8 +269,8 @@ class Test_Files_Sharing_Cache extends Test_Files_Sharing_Base { \OC_Util::tearDownFS(); self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - $this->assertTrue(\OC\Files\Filesystem::file_exists('/Shared/foo')); - list($sharedStorage) = \OC\Files\Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/Shared/foo'); + $this->assertTrue(\OC\Files\Filesystem::file_exists('/foo')); + list($sharedStorage) = \OC\Files\Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/foo'); /** * @var \OC\Files\Storage\Shared $sharedStorage */ diff --git a/apps/files_sharing/tests/permissions.php b/apps/files_sharing/tests/permissions.php index e301d384a4..5ac251b052 100644 --- a/apps/files_sharing/tests/permissions.php +++ b/apps/files_sharing/tests/permissions.php @@ -23,6 +23,9 @@ require_once __DIR__ . '/base.php'; class Test_Files_Sharing_Permissions extends Test_Files_Sharing_Base { + private $sharedStorageRestrictedShare; + private $sharedCacheRestrictedShare; + function setUp() { parent::setUp(); @@ -55,8 +58,10 @@ class Test_Files_Sharing_Permissions extends Test_Files_Sharing_Base { // retrieve the shared storage $this->secondView = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2); - list($this->sharedStorage, $internalPath) = $this->secondView->resolvePath('files/Shared/shareddir'); + list($this->sharedStorage, $internalPath) = $this->secondView->resolvePath('files/shareddir'); + list($this->sharedStorageRestrictedShare, $internalPath) = $this->secondView->resolvePath('files/shareddirrestricted'); $this->sharedCache = $this->sharedStorage->getCache(); + $this->sharedCacheRestrictedShare = $this->sharedStorageRestrictedShare->getCache(); } function tearDown() { @@ -86,9 +91,9 @@ class Test_Files_Sharing_Permissions extends Test_Files_Sharing_Base { $this->assertEquals(31, $sharedDirPerms); $sharedDirPerms = $this->sharedStorage->getPermissions('shareddir/textfile.txt'); $this->assertEquals(31, $sharedDirPerms); - $sharedDirRestrictedPerms = $this->sharedStorage->getPermissions('shareddirrestricted'); + $sharedDirRestrictedPerms = $this->sharedStorageRestrictedShare->getPermissions('shareddirrestricted'); $this->assertEquals(7, $sharedDirRestrictedPerms); - $sharedDirRestrictedPerms = $this->sharedStorage->getPermissions('shareddirrestricted/textfile.txt'); + $sharedDirRestrictedPerms = $this->sharedStorageRestrictedShare->getPermissions('shareddirrestricted/textfile.txt'); $this->assertEquals(7, $sharedDirRestrictedPerms); } @@ -96,12 +101,12 @@ class Test_Files_Sharing_Permissions extends Test_Files_Sharing_Base { * Test that the permissions of shared directory are returned correctly */ function testGetDirectoryPermissions() { - $contents = $this->secondView->getDirectoryContent('files/Shared/shareddir'); + $contents = $this->secondView->getDirectoryContent('files/shareddir'); $this->assertEquals('subdir', $contents[0]['name']); $this->assertEquals(31, $contents[0]['permissions']); $this->assertEquals('textfile.txt', $contents[1]['name']); $this->assertEquals(31, $contents[1]['permissions']); - $contents = $this->secondView->getDirectoryContent('files/Shared/shareddirrestricted'); + $contents = $this->secondView->getDirectoryContent('files/shareddirrestricted'); $this->assertEquals('subdir', $contents[0]['name']); $this->assertEquals(7, $contents[0]['permissions']); $this->assertEquals('textfile1.txt', $contents[1]['name']); diff --git a/apps/files_sharing/tests/watcher.php b/apps/files_sharing/tests/watcher.php index 5ab716e829..bce93c80a6 100644 --- a/apps/files_sharing/tests/watcher.php +++ b/apps/files_sharing/tests/watcher.php @@ -48,7 +48,7 @@ class Test_Files_Sharing_Watcher extends Test_Files_Sharing_Base { // retrieve the shared storage $secondView = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2); - list($this->sharedStorage, $internalPath) = $secondView->resolvePath('files/Shared/shareddir'); + list($this->sharedStorage, $internalPath) = $secondView->resolvePath('files/shareddir'); $this->sharedCache = $this->sharedStorage->getCache(); } @@ -77,12 +77,12 @@ class Test_Files_Sharing_Watcher extends Test_Files_Sharing_Base { $textData = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'; $dataLen = strlen($textData); - $this->sharedCache->put('shareddir/bar.txt', array('storage_mtime' => 10)); - $this->sharedStorage->file_put_contents('shareddir/bar.txt', $textData); - $this->sharedCache->put('shareddir', array('storage_mtime' => 10)); + $this->sharedCache->put('bar.txt', array('mtime' => 10, 'storage_mtime' => 10, 'size' => $dataLen, 'mimetype' => 'text/plain')); + $this->sharedStorage->file_put_contents('bar.txt', $textData); + $this->sharedCache->put('', array('mtime' => 10, 'storage_mtime' => 10, 'size' => '-1', 'mimetype' => 'httpd/unix-directory')); // run the propagation code - $result = $this->sharedStorage->getWatcher()->checkUpdate('shareddir'); + $result = $this->sharedStorage->getWatcher()->checkUpdate(''); $this->assertTrue($result); @@ -94,7 +94,7 @@ class Test_Files_Sharing_Watcher extends Test_Files_Sharing_Base { $this->assertEquals($initialSizes['files/container/shareddir'] + $dataLen, $newSizes['files/container/shareddir']); // no more updates - $result = $this->sharedStorage->getWatcher()->checkUpdate('shareddir'); + $result = $this->sharedStorage->getWatcher()->checkUpdate(''); $this->assertFalse($result); } @@ -108,12 +108,12 @@ class Test_Files_Sharing_Watcher extends Test_Files_Sharing_Base { $textData = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'; $dataLen = strlen($textData); - $this->sharedCache->put('shareddir/subdir/bar.txt', array('storage_mtime' => 10)); - $this->sharedStorage->file_put_contents('shareddir/subdir/bar.txt', $textData); - $this->sharedCache->put('shareddir/subdir', array('storage_mtime' => 10)); + $this->sharedCache->put('subdir/bar.txt', array('mtime' => 10, 'storage_mtime' => 10, 'size' => $dataLen, 'mimetype' => 'text/plain')); + $this->sharedStorage->file_put_contents('subdir/bar.txt', $textData); + $this->sharedCache->put('subdir', array('mtime' => 10, 'storage_mtime' => 10, 'size' => $dataLen, 'mimetype' => 'text/plain')); // run the propagation code - $result = $this->sharedStorage->getWatcher()->checkUpdate('shareddir/subdir'); + $result = $this->sharedStorage->getWatcher()->checkUpdate('subdir'); $this->assertTrue($result); @@ -126,22 +126,11 @@ class Test_Files_Sharing_Watcher extends Test_Files_Sharing_Base { $this->assertEquals($initialSizes['files/container/shareddir/subdir'] + $dataLen, $newSizes['files/container/shareddir/subdir']); // no more updates - $result = $this->sharedStorage->getWatcher()->checkUpdate('shareddir/subdir'); + $result = $this->sharedStorage->getWatcher()->checkUpdate('subdir'); $this->assertFalse($result); } - function testNoUpdateOnRoot() { - // no updates when called for root path - $result = $this->sharedStorage->getWatcher()->checkUpdate(''); - - $this->assertFalse($result); - - // FIXME: for some reason when running this "naked" test, - // there will be remaining nonsensical entries in the - // database with a path "test-share-user1/container/..." - } - /** * Returns the sizes of the path and its parent dirs in a hash * where the key is the path and the value is the size. diff --git a/lib/private/share/share.php b/lib/private/share/share.php index ff56b9a48f..24e2a15064 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -1177,10 +1177,6 @@ class Share extends \OC\Share\Constants { // Remove root from file source paths if retrieving own shared items if (isset($uidOwner) && isset($row['path'])) { if (isset($row['parent'])) { - // FIXME: Doesn't always construct the correct path, example: - // Folder '/a/b', share '/a' and '/a/b' to user2 - // user2 reshares /Shared/b and ask for share status of /Shared/a/b - // expected result: path=/Shared/a/b; actual result /Shared/b because of the parent $query = \OC_DB::prepare('SELECT `file_target` FROM `*PREFIX*share` WHERE `id` = ?'); $parentResult = $query->execute(array($row['parent'])); if (\OC_DB::isError($result)) { @@ -1189,7 +1185,7 @@ class Share extends \OC\Share\Constants { \OC_Log::ERROR); } else { $parentRow = $parentResult->fetchRow(); - $tmpPath = '/Shared' . $parentRow['file_target']; + $tmpPath = $parentRow['file_target']; // find the right position where the row path continues from the target path $pos = strrpos($row['path'], $parentRow['file_target']); $subPath = substr($row['path'], $pos); From 4c840cb61d560ae567e3e9aaa7bd411cac917e48 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 9 Apr 2014 17:51:54 +0200 Subject: [PATCH 14/34] fix target generation for group shares --- lib/private/share/share.php | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/private/share/share.php b/lib/private/share/share.php index 24e2a15064..7af68d1b25 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -1395,8 +1395,8 @@ class Share extends \OC\Share\Constants { } } $query = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (`item_type`, `item_source`, `item_target`,' - .' `parent`, `share_type`, `share_with`, `uid_owner`, `permissions`, `stime`, `file_source`,' - .' `file_target`, `token`) VALUES (?,?,?,?,?,?,?,?,?,?,?,?)'); + .' `share_type`, `share_with`, `uid_owner`, `permissions`, `stime`, `file_source`,' + .' `file_target`, `token`, `parent`) VALUES (?,?,?,?,?,?,?,?,?,?,?,?)'); // Share with a group if ($shareType == self::SHARE_TYPE_GROUP) { $groupItemTarget = Helper::generateTarget($itemType, $itemSource, $shareType, $shareWith['group'], @@ -1440,10 +1440,9 @@ class Share extends \OC\Share\Constants { } else { $groupFileTarget = null; } - $query->execute(array($itemType, $itemSource, $groupItemTarget, $parent, $shareType, - $shareWith['group'], $uidOwner, $permissions, time(), $fileSource, $groupFileTarget, $token)); - // Save this id, any extra rows for this group share will need to reference it - $parent = \OC_DB::insertid('*PREFIX*share'); + $queriesToExecute = array(); + $queriesToExecute['groupShare'] = array($itemType, $itemSource, $groupItemTarget, $shareType, + $shareWith['group'], $uidOwner, $permissions, time(), $fileSource, $groupFileTarget, $token, $parent); // Loop through all users of this group in case we need to add an extra row foreach ($shareWith['users'] as $uid) { $itemTarget = Helper::generateTarget($itemType, $itemSource, self::SHARE_TYPE_USER, $uid, @@ -1469,12 +1468,21 @@ class Share extends \OC\Share\Constants { } // Insert an extra row for the group share if the item or file target is unique for this user if ($itemTarget != $groupItemTarget || (isset($fileSource) && $fileTarget != $groupFileTarget)) { - $query->execute(array($itemType, $itemSource, $itemTarget, $parent, + $queriesToExecute[] = array($itemType, $itemSource, $itemTarget, self::$shareTypeGroupUserUnique, $uid, $uidOwner, $permissions, time(), - $fileSource, $fileTarget, $token)); + $fileSource, $fileTarget, $token); $id = \OC_DB::insertid('*PREFIX*share'); } } + $query->execute($queriesToExecute['groupShare']); + // Save this id, any extra rows for this group share will need to reference it + $parent = \OC_DB::insertid('*PREFIX*share'); + unset($queriesToExecute['groupShare']); + foreach ($queriesToExecute as $qe) { + $qe[] = $parent; + $query->execute($qe); + } + \OC_Hook::emit('OCP\Share', 'post_shared', array( 'itemType' => $itemType, 'itemSource' => $itemSource, @@ -1534,8 +1542,8 @@ class Share extends \OC\Share\Constants { } else { $fileTarget = null; } - $query->execute(array($itemType, $itemSource, $itemTarget, $parent, $shareType, $shareWith, $uidOwner, - $permissions, time(), $fileSource, $fileTarget, $token)); + $query->execute(array($itemType, $itemSource, $itemTarget, $shareType, $shareWith, $uidOwner, + $permissions, time(), $fileSource, $fileTarget, $token, $parent)); $id = \OC_DB::insertid('*PREFIX*share'); \OC_Hook::emit('OCP\Share', 'post_shared', array( 'itemType' => $itemType, From a86d97295e4e0e1560bcd4cce4bde21aa60a2486 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 9 Apr 2014 17:52:24 +0200 Subject: [PATCH 15/34] fix encryption tests after the removal of the shared folder --- apps/files_encryption/hooks/hooks.php | 22 +++++----- apps/files_encryption/lib/util.php | 5 +-- apps/files_encryption/tests/hooks.php | 26 ++++-------- apps/files_encryption/tests/share.php | 51 ++++++++++++------------ apps/files_encryption/tests/util.php | 2 +- apps/files_sharing/lib/share/file.php | 6 +++ apps/files_sharing/lib/sharedstorage.php | 14 +++---- 7 files changed, 59 insertions(+), 67 deletions(-) diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php index 0b6c5adf3f..7a8b54e2d7 100644 --- a/apps/files_encryption/hooks/hooks.php +++ b/apps/files_encryption/hooks/hooks.php @@ -342,9 +342,7 @@ class Hooks { // if parent has the same type than the child it is a 1:1 share if ($parent['item_type'] === $params['itemType']) { - - // prefix path with Shared - $path = '/Shared' . $parent['file_target']; + $path = $parent['file_target']; } else { // NOTE: parent is folder but shared was a file! @@ -376,11 +374,9 @@ class Hooks { break; } } - // prefix path with Shared - $path = '/Shared' . $parent['file_target'] . $path; + $path = $parent['file_target'] . $path; } else { - // prefix path with Shared - $path = '/Shared' . $parent['file_target'] . $params['fileTarget']; + $path = $parent['file_target'] . $params['fileTarget']; } } } @@ -388,7 +384,8 @@ class Hooks { $sharingEnabled = \OCP\Share::isEnabled(); // get the path including mount point only if not a shared folder - if (strncmp($path, '/Shared', strlen('/Shared') !== 0)) { + list($storage, ) = \OC\Files\Filesystem::resolvePath('/' . $userId . '/files' . $path); + if (!($storage instanceof \OC\Files\Storage\Shared)) { // get path including the the storage mount point $path = $util->getPathWithMountPoint($params['itemSource']); } @@ -454,7 +451,7 @@ class Hooks { } // prefix path with Shared - $path = '/Shared' . $parent['file_target'] . $path; + $path = $parent['file_target'] . $path; } // for group shares get a list of the group members @@ -469,9 +466,10 @@ class Hooks { } // get the path including mount point only if not a shared folder - if (strncmp($path, '/Shared', strlen('/Shared') !== 0)) { + list($storage, ) = \OC\Files\Filesystem::resolvePath($path); + if (!($storage instanceof \OC\Files\Storage\Shared)) { // get path including the the storage mount point - $path = $util->getPathWithMountPoint($params['itemSource']); + //$path = $util->getPathWithMountPoint($params['itemSource']); } // if we unshare a folder we need a list of all (sub-)files @@ -510,6 +508,8 @@ class Hooks { // otherwise we perform a stream copy, so we get a new set of keys $mp1 = $view->getMountPoint('/' . $user . '/files/' . $params['oldpath']); $mp2 = $view->getMountPoint('/' . $user . '/files/' . $params['newpath']); + list($storage1, ) = Filesystem::resolvePath($params['oldpath']); + if ($mp1 === $mp2) { self::$renamedFiles[$params['oldpath']] = array( 'uid' => $ownerOld, diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php index b86815021a..4be4ab9565 100644 --- a/apps/files_encryption/lib/util.php +++ b/apps/files_encryption/lib/util.php @@ -1408,11 +1408,10 @@ class Util { $this->userFilesDir . '/' . $dir)); foreach ($content as $c) { - $usersPath = isset($c['usersPath']) ? $c['usersPath'] : $c['path']; if ($c['type'] === 'dir') { - $dirList[] = substr($usersPath, strlen("files")); + $dirList[] = substr($c['path'], strlen("files")); } else { - $result[] = substr($usersPath, strlen("files")); + $result[] = substr($c['path'], strlen("files")); } } diff --git a/apps/files_encryption/tests/hooks.php b/apps/files_encryption/tests/hooks.php index d0e4b5f732..047084ca2c 100644 --- a/apps/files_encryption/tests/hooks.php +++ b/apps/files_encryption/tests/hooks.php @@ -219,18 +219,20 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase { \Test_Encryption_Util::loginHelper(\Test_Encryption_Hooks::TEST_ENCRYPTION_HOOKS_USER2); \OC_User::setUserId(\Test_Encryption_Hooks::TEST_ENCRYPTION_HOOKS_USER2); - // user2 has a local file with the same name + // user2 update the shared file $this->user2View->file_put_contents($this->filename, $this->data); - // check if all keys are generated - $this->assertTrue($this->rootView->file_exists( + // keys should be stored at user1s dir, not in user2s + $this->assertFalse($this->rootView->file_exists( self::TEST_ENCRYPTION_HOOKS_USER2 . '/files_encryption/share-keys/' . $this->filename . '.' . \Test_Encryption_Hooks::TEST_ENCRYPTION_HOOKS_USER2 . '.shareKey')); - $this->assertTrue($this->rootView->file_exists( + $this->assertFalse($this->rootView->file_exists( self::TEST_ENCRYPTION_HOOKS_USER2 . '/files_encryption/keyfiles/' . $this->filename . '.key')); // delete the Shared file from user1 in data/user2/files/Shared - $this->user2View->unlink('/Shared/' . $this->filename); + $result = $this->user2View->unlink($this->filename); + + $this->assertTrue($result); // now keys from user1s home should be gone $this->assertFalse($this->rootView->file_exists( @@ -242,26 +244,12 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase { $this->assertFalse($this->rootView->file_exists( self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/' . $this->filename . '.key')); - // but user2 keys should still exist - $this->assertTrue($this->rootView->file_exists( - self::TEST_ENCRYPTION_HOOKS_USER2 . '/files_encryption/share-keys/' - . $this->filename . '.' . \Test_Encryption_Hooks::TEST_ENCRYPTION_HOOKS_USER2 . '.shareKey')); - $this->assertTrue($this->rootView->file_exists( - self::TEST_ENCRYPTION_HOOKS_USER2 . '/files_encryption/keyfiles/' . $this->filename . '.key')); - // cleanup - $this->user2View->unlink($this->filename); - \Test_Encryption_Util::logoutHelper(); \Test_Encryption_Util::loginHelper(\Test_Encryption_Hooks::TEST_ENCRYPTION_HOOKS_USER1); \OC_User::setUserId(\Test_Encryption_Hooks::TEST_ENCRYPTION_HOOKS_USER1); - // unshare the file - \OCP\Share::unshare('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_ENCRYPTION_HOOKS_USER2); - - $this->user1View->unlink($this->filename); - if ($stateFilesTrashbin) { OC_App::enable('files_trashbin'); } diff --git a/apps/files_encryption/tests/share.php b/apps/files_encryption/tests/share.php index 1f57d7cb63..512671c576 100755 --- a/apps/files_encryption/tests/share.php +++ b/apps/files_encryption/tests/share.php @@ -175,7 +175,7 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // get file contents $retrievedCryptedFile = $this->view->file_get_contents( - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/Shared/' . $this->filename); + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->filename); // check if data is the same as we previously written $this->assertEquals($this->dataShort, $retrievedCryptedFile); @@ -213,14 +213,14 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { function testReShareFile($withTeardown = true) { $this->testShareFile(false); - // login as user1 + // login as user2 \Test_Encryption_Util::loginHelper(\Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2); // get the file info $fileInfo = $this->view->getFileInfo( - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/Shared/' . $this->filename); + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->filename); - // share the file with user2 + // share the file with user3 \OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3, OCP\PERMISSION_ALL); // login as admin @@ -236,7 +236,7 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // get file contents $retrievedCryptedFile = $this->view->file_get_contents( - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3 . '/files/Shared/' . $this->filename); + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3 . '/files/' . $this->filename); // check if data is the same as previously written $this->assertEquals($this->dataShort, $retrievedCryptedFile); @@ -333,7 +333,7 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // get file contents $retrievedCryptedFile = $this->view->file_get_contents( - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/Shared' . $this->folder1 + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->folder1 . $this->subfolder . $this->subsubfolder . '/' . $this->filename); // check if data is the same @@ -376,7 +376,7 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { function testReShareFolder($withTeardown = true) { $fileInfoFolder1 = $this->testShareFolder(false); - // login as user1 + // login as user2 \Test_Encryption_Util::loginHelper(\Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2); // disable encryption proxy to prevent recursive calls @@ -385,7 +385,7 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // get the file info from previous created folder $fileInfoSubFolder = $this->view->getFileInfo( - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/Shared' . $this->folder1 + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->folder1 . $this->subfolder); // check if we have a valid file info @@ -394,24 +394,24 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // re-enable the file proxy \OC_FileProxy::$enabled = $proxyStatus; - // share the file with user2 + // share the file with user3 \OCP\Share::shareItem('folder', $fileInfoSubFolder['fileid'], \OCP\Share::SHARE_TYPE_USER, \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3, OCP\PERMISSION_ALL); // login as admin \Test_Encryption_Util::loginHelper(\Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1); - // check if share key for user2 exists + // check if share key for user3 exists $this->assertTrue($this->view->file_exists( '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '/files_encryption/share-keys' . $this->folder1 . $this->subfolder . $this->subsubfolder . '/' . $this->filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3 . '.shareKey')); - // login as user2 + // login as user3 \Test_Encryption_Util::loginHelper(\Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3); // get file contents $retrievedCryptedFile = $this->view->file_get_contents( - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3 . '/files/Shared' . $this->subfolder + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3 . '/files/' . $this->subfolder . $this->subsubfolder . '/' . $this->filename); // check if data is the same @@ -419,7 +419,7 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // get the file info $fileInfo = $this->view->getFileInfo( - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3 . '/files/Shared' . $this->subfolder + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3 . '/files/' . $this->subfolder . $this->subsubfolder . '/' . $this->filename); // check if we have fileInfos @@ -442,7 +442,7 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // get file contents $retrievedCryptedFile = $this->view->file_get_contents( - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER4 . '/files/Shared/' . $this->filename); + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER4 . '/files/' . $this->filename); // check if data is the same $this->assertEquals($this->dataShort, $retrievedCryptedFile); @@ -624,7 +624,7 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // get file contents $retrievedCryptedFile = $this->view->file_get_contents( - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3 . '/files/Shared/' . $this->filename); + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER3 . '/files/' . $this->filename); // check if data is the same as we previously written $this->assertEquals($this->dataShort, $retrievedCryptedFile); @@ -676,6 +676,7 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // enable recovery for admin $this->assertTrue($util->setRecoveryForUser(1)); + $util->addRecoveryKeys(); // create folder structure $this->view->mkdir('/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '/files' . $this->folder1); @@ -981,7 +982,7 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // share the file \OCP\Share::shareItem('file', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2, OCP\PERMISSION_ALL); - // check if share key for user2exists + // check if share key for user2 exists $this->assertTrue($this->view->file_exists( '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '/files_encryption/share-keys/' . $this->filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey')); @@ -990,31 +991,29 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // login as user2 \Test_Encryption_Util::loginHelper(\Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2); - $this->assertTrue($this->view->file_exists('/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/Shared/' . $this->filename)); + $this->assertTrue($this->view->file_exists('/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->filename)); // get file contents $retrievedCryptedFile = $this->view->file_get_contents( - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/Shared/' . $this->filename); + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->filename); // check if data is the same as we previously written $this->assertEquals($this->dataShort, $retrievedCryptedFile); - // move the file out of the shared folder - $this->view->rename('/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/Shared/' . $this->filename, - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->filename); + // move the file to a subfolder + $this->view->rename('/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->filename, + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->folder1 . $this->filename); // check if we can read the moved file $retrievedRenamedFile = $this->view->file_get_contents( - '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->filename); + '/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->folder1 . $this->filename); // check if data is the same as we previously written $this->assertEquals($this->dataShort, $retrievedRenamedFile); - // the owners file should be deleted - $this->assertFalse($this->view->file_exists('/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename)); - // cleanup - $this->view->unlink('/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '/files/' . $this->filename); + \Test_Encryption_Util::loginHelper(\Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1); + $this->view->unlink('/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '/files/' . $this->filename); } } diff --git a/apps/files_encryption/tests/util.php b/apps/files_encryption/tests/util.php index 203ba55dbf..717b66b86a 100755 --- a/apps/files_encryption/tests/util.php +++ b/apps/files_encryption/tests/util.php @@ -520,8 +520,8 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase { \OC_Util::tearDownFS(); \OC_User::setUserId(''); \OC\Files\Filesystem::tearDown(); - \OC_Util::setupFS($user); \OC_User::setUserId($user); + \OC_Util::setupFS($user); $params['uid'] = $user; $params['password'] = $password; diff --git a/apps/files_sharing/lib/share/file.php b/apps/files_sharing/lib/share/file.php index e1d0b49706..c0c9e0c107 100644 --- a/apps/files_sharing/lib/share/file.php +++ b/apps/files_sharing/lib/share/file.php @@ -60,6 +60,12 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { */ public function generateTarget($filePath, $shareWith, $exclude = null) { $target = '/'.basename($filePath); + + // for group shares we return the target right away + if ($shareWith === false) { + return $target; + } + \OC\Files\Filesystem::initMountPoints($shareWith); $view = new \OC\Files\View('/' . $shareWith . '/files'); $excludeList = \OCP\Share::getItemsSharedWithUser('file', $shareWith, self::FORMAT_TARGET_NAMES); diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 565c3e7af8..c90f6d71f7 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -195,7 +195,7 @@ class Shared extends \OC\Files\Storage\Common { public function isCreatable($path) { if ($path == '') { - return ($this->getPermissions($this->getMountPoint()) & \OCP\PERMISSION_CREATE); + $path = $this->mountPoint; } return ($this->getPermissions($path) & \OCP\PERMISSION_CREATE); } @@ -206,21 +206,21 @@ class Shared extends \OC\Files\Storage\Common { public function isUpdatable($path) { if ($path == '') { - return false; + $path = $this->mountPoint; } return ($this->getPermissions($path) & \OCP\PERMISSION_UPDATE); } public function isDeletable($path) { if ($path == '') { - return true; + $path = $this->mountPoint; } return ($this->getPermissions($path) & \OCP\PERMISSION_DELETE); } public function isSharable($path) { if ($path == '') { - return false; + $path = $this->mountPoint; } return ($this->getPermissions($path) & \OCP\PERMISSION_SHARE); } @@ -441,7 +441,7 @@ class Shared extends \OC\Files\Storage\Common { public function free_space($path) { if ($path == '') { - return \OC\Files\SPACE_UNKNOWN; + $path = $this->mountPoint; } $source = $this->getSourcePath($path); if ($source) { @@ -531,7 +531,7 @@ class Shared extends \OC\Files\Storage\Common { public function getOwner($path) { if ($path == '') { - return false; + $path = $this->mountPoint; } $source = $this->getFile($path); if ($source) { @@ -542,7 +542,7 @@ class Shared extends \OC\Files\Storage\Common { public function getETag($path) { if ($path == '') { - return parent::getETag($path); + $path = $this->mountPoint; } if ($source = $this->getSourcePath($path)) { list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); From bfabd247f42f306720d154c3a84285afed589033 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 9 Apr 2014 18:00:00 +0200 Subject: [PATCH 16/34] fix updating of shared files --- apps/files_sharing/lib/sharedstorage.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index c90f6d71f7..39d0140008 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -144,13 +144,9 @@ class Shared extends \OC\Files\Storage\Common { } public function is_dir($path) { - if ($path == '' || $path == '/') { - return true; - } else if ($source = $this->getSourcePath($path)) { - list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); - return $storage->is_dir($internalPath); - } - return false; + $source = $this->getSourcePath($path); + list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); + return $storage->is_dir($internalPath); } public function is_file($path) { From 33cdd938904d92607db56c1f3993b74b14ce9bf3 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 10 Apr 2014 09:54:29 +0200 Subject: [PATCH 17/34] fix deleting of shared files --- apps/files_sharing/lib/sharedstorage.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 39d0140008..3a31e96554 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -272,6 +272,9 @@ class Shared extends \OC\Files\Storage\Common { public function unlink($path) { // Delete the file if DELETE permission is granted + if ($path == '') { + $path = $this->mountPoint; + } if ($source = $this->getSourcePath($path)) { if ($this->isDeletable($path)) { list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); From d4085d81485c3d973faf4211e11af1c1853e619c Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 11 Apr 2014 11:18:50 +0200 Subject: [PATCH 18/34] make sure that path is not 'false' --- apps/files_sharing/lib/cache.php | 6 +++--- apps/files_sharing/lib/sharedstorage.php | 4 +--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index e91c15cc62..11f180bf7e 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -169,9 +169,8 @@ class Shared_Cache extends Cache { * @return int file id */ public function put($file, array $data) { - if ($file === '' && isset($data['etag'])) { - return \OCP\Config::setUserValue(\OCP\User::getUser(), 'files_sharing', 'etag', $data['etag']); - } else if ($cache = $this->getSourceCache($file)) { + $file = ($file === false) ? '' : $file; + if ($cache = $this->getSourceCache($file)) { return $cache->put($this->files[$file], $data); } return false; @@ -368,6 +367,7 @@ class Shared_Cache extends Cache { * @return int */ public function calculateFolderSize($path, $entry = null) { + $path = ($path === false) ? '' : $path; if ($cache = $this->getSourceCache($path)) { return $cache->calculateFolderSize($this->files[$path]); } diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 3a31e96554..8254f0e05c 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -272,9 +272,7 @@ class Shared extends \OC\Files\Storage\Common { public function unlink($path) { // Delete the file if DELETE permission is granted - if ($path == '') { - $path = $this->mountPoint; - } + $path = ($path === false) ? '' : $path; if ($source = $this->getSourcePath($path)) { if ($this->isDeletable($path)) { list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); From dfb69e9418704d8553193676f3a2ae85697c0fa9 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 11 Apr 2014 11:40:14 +0200 Subject: [PATCH 19/34] allow user to delete shared files/folders --- lib/private/files/view.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 6d630f978b..3f73632be1 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -348,7 +348,8 @@ class View { $postFix = (substr($path, -1, 1) === '/') ? '/' : ''; $absolutePath = Filesystem::normalizePath($this->getAbsolutePath($path)); list($storage, $internalPath) = Filesystem::resolvePath($absolutePath . $postFix); - if (!$internalPath || $internalPath === '' || $internalPath === '/') { + if (!($storage instanceof \OC\Files\Storage\Shared) && + (!$internalPath || $internalPath === '' || $internalPath === '/')) { // do not allow deleting the storage's root / the mount point // because for some storages it might delete the whole contents // but isn't supposed to work that way From c9bd2f7a6c1e72a2ed6d7aa5f180e2c0757ac24c Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 11 Apr 2014 11:54:14 +0200 Subject: [PATCH 20/34] also fetch the etag from file cache --- apps/files_sharing/lib/cache.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index 11f180bf7e..bce22a74ba 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -102,7 +102,7 @@ class Shared_Cache extends Cache { } $query = \OC_DB::prepare( 'SELECT `fileid`, `storage`, `path`, `parent`, `name`, `mimetype`, `mimepart`,' - . ' `size`, `mtime`, `encrypted`, `unencrypted_size`, `storage_mtime`' + . ' `size`, `mtime`, `encrypted`, `unencrypted_size`, `storage_mtime`, `etag`' . ' FROM `*PREFIX*filecache` WHERE `fileid` = ?'); $result = $query->execute(array($file)); $data = $result->fetchRow(); @@ -212,6 +212,7 @@ class Shared_Cache extends Cache { * @param string $file */ public function remove($file) { + $file = ($file === false) ? '' : $file; if ($cache = $this->getSourceCache($file)) { $cache->remove($this->files[$file]); } From 22e0a4b9a8835df4bfc88b478e694b0983cb1060 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 14 Apr 2014 10:15:43 +0200 Subject: [PATCH 21/34] external storages: allow to mount a folder called "Shared", it is no longer a reserved name --- apps/files_external/lib/config.php | 4 ++-- apps/files_external/tests/mountconfig.php | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php index 71f6ae7887..99eca2f38c 100755 --- a/apps/files_external/lib/config.php +++ b/apps/files_external/lib/config.php @@ -373,8 +373,8 @@ class OC_Mount_Config { $isPersonal = false) { $backends = self::getBackends(); $mountPoint = OC\Files\Filesystem::normalizePath($mountPoint); - if ($mountPoint === '' || $mountPoint === '/' || $mountPoint == '/Shared') { - // can't mount at root or "Shared" folder + if ($mountPoint === '' || $mountPoint === '/') { + // can't mount at root folder return false; } diff --git a/apps/files_external/tests/mountconfig.php b/apps/files_external/tests/mountconfig.php index c89874c94d..1921ec76af 100644 --- a/apps/files_external/tests/mountconfig.php +++ b/apps/files_external/tests/mountconfig.php @@ -128,9 +128,6 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { $isPersonal = false; $this->assertFalse(OC_Mount_Config::addMountPoint('', $storageClass, array(), $mountType, $applicable, $isPersonal)); $this->assertFalse(OC_Mount_Config::addMountPoint('/', $storageClass, array(), $mountType, $applicable, $isPersonal)); - $this->assertFalse(OC_Mount_Config::addMountPoint('Shared', $storageClass, array(), $mountType, $applicable, $isPersonal)); - $this->assertFalse(OC_Mount_Config::addMountPoint('/Shared', $storageClass, array(), $mountType, $applicable, $isPersonal)); - } /** @@ -488,7 +485,7 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase { 'root' => 'someroot' ); - // add mount point as "test" user + // add mount point as "test" user $this->assertTrue( OC_Mount_Config::addMountPoint( '/ext', From 3f6e9e01026031527cd80f1f608d7a98d93e7ebe Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 14 Apr 2014 11:33:10 +0200 Subject: [PATCH 22/34] cleanup the shared storage, always keep the whole share resource --- apps/files_sharing/lib/cache.php | 8 +-- apps/files_sharing/lib/sharedstorage.php | 74 +++++++++++++----------- 2 files changed, 43 insertions(+), 39 deletions(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index bce22a74ba..7e44847e40 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -50,7 +50,7 @@ class Shared_Cache extends Cache { if ($target === false || $target === $this->storage->getMountPoint()) { $target = ''; } - $source = \OC_Share_Backend_File::getSource($target, $this->storage->getMountPoint(), $this->storage->getShareType()); + $source = \OC_Share_Backend_File::getSource($target, $this->storage->getMountPoint(), $this->storage->getItemType()); if (isset($source['path']) && isset($source['fileOwner'])) { \OC\Files\Filesystem::initMountPoints($source['fileOwner']); $mount = \OC\Files\Filesystem::getMountByNumericId($source['storage']); @@ -86,7 +86,7 @@ class Shared_Cache extends Cache { public function get($file) { if (is_string($file)) { if ($cache = $this->getSourceCache($file)) { - $path = 'files/' . $this->storage->getMountPoint(); + $path = 'files' . $this->storage->getMountPoint(); $path .= ($file !== '') ? '/' . $file : ''; $data = $cache->get($this->files[$file]); $data['displayname_owner'] = \OC_User::getDisplayName($this->storage->getSharedFrom()); @@ -141,7 +141,7 @@ class Shared_Cache extends Cache { $folder = ''; } - $dir = 'files/' . $this->storage->getMountPoint(); + $dir = 'files' . $this->storage->getMountPoint(); $dir .= ($folder !== '') ? '/' . $folder : ''; $cache = $this->getSourceCache($folder); @@ -226,7 +226,7 @@ class Shared_Cache extends Cache { */ public function move($source, $target) { if ($cache = $this->getSourceCache($source)) { - $file = \OC_Share_Backend_File::getSource($target, $this->storage->getMountPoint(), $this->storage->getShareType()); + $file = \OC_Share_Backend_File::getSource($target, $this->storage->getMountPoint(), $this->storage->getItemType()); if ($file && isset($file['path'])) { $cache->move($this->files[$source], $file['path']); } diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 8254f0e05c..7ce9dd58b4 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -28,19 +28,11 @@ namespace OC\Files\Storage; */ class Shared extends \OC\Files\Storage\Common { - private $mountPoint; // mount point relative to data/user/files - private $type; // can be "file" or "folder" - private $shareId; // share Id to identify the share in the database - private $fileSource; // file cache ID of the shared item - private $sharedFrom; // the user who shared the file + private $share; // the shared resource private $files = array(); public function __construct($arguments) { - $this->mountPoint = $arguments['shareTarget']; - $this->type = $arguments['shareType']; - $this->shareId = $arguments['shareId']; - $this->fileSource = $arguments['fileSource']; - $this->sharedFrom = $arguments['sharedFrom']; + $this->share = $arguments['share']; } /** @@ -48,7 +40,7 @@ class Shared extends \OC\Files\Storage\Common { * @return string */ public function getId() { - return 'shared::' . $this->mountPoint; + return 'shared::' . $this->getMountPoint(); } /** @@ -56,7 +48,7 @@ class Shared extends \OC\Files\Storage\Common { * @return string */ public function getSourceId() { - return $this->fileSource; + return $this->share['file_source']; } /** @@ -69,14 +61,14 @@ class Shared extends \OC\Files\Storage\Common { if (!isset($this->files[$target])) { // Check for partial files if (pathinfo($target, PATHINFO_EXTENSION) === 'part') { - $source = \OC_Share_Backend_File::getSource(substr($target, 0, -5), $this->getMountPoint(), $this->getShareType()); + $source = \OC_Share_Backend_File::getSource(substr($target, 0, -5), $this->getMountPoint(), $this->getItemType()); if ($source) { $source['path'] .= '.part'; // All partial files have delete permission $source['permissions'] |= \OCP\PERMISSION_DELETE; } } else { - $source = \OC_Share_Backend_File::getSource($target, $this->getMountPoint(), $this->getShareType()); + $source = \OC_Share_Backend_File::getSource($target, $this->getMountPoint(), $this->getItemType()); } $this->files[$target] = $source; } @@ -191,7 +183,7 @@ class Shared extends \OC\Files\Storage\Common { public function isCreatable($path) { if ($path == '') { - $path = $this->mountPoint; + $path = $this->getMountPoint(); } return ($this->getPermissions($path) & \OCP\PERMISSION_CREATE); } @@ -202,21 +194,21 @@ class Shared extends \OC\Files\Storage\Common { public function isUpdatable($path) { if ($path == '') { - $path = $this->mountPoint; + $path = $this->getMountPoint(); } return ($this->getPermissions($path) & \OCP\PERMISSION_UPDATE); } public function isDeletable($path) { if ($path == '') { - $path = $this->mountPoint; + $path = $this->getMountPoint(); } return ($this->getPermissions($path) & \OCP\PERMISSION_DELETE); } public function isSharable($path) { if ($path == '') { - $path = $this->mountPoint; + $path = $this->getMountPoint(); } return ($this->getPermissions($path) & \OCP\PERMISSION_SHARE); } @@ -241,7 +233,7 @@ class Shared extends \OC\Files\Storage\Common { $source = $this->getSourcePath($path); if ($source) { $info = array( - 'target' => $this->mountPoint . $path, + 'target' => $this->getMountPoint() . $path, 'source' => $source, ); \OCP\Util::emitHook('\OC\Files\Storage\Shared', 'file_get_contents', $info); @@ -259,7 +251,7 @@ class Shared extends \OC\Files\Storage\Common { return false; } $info = array( - 'target' => $this->mountPoint . '/' . $path, + 'target' => $this->getMountPoint() . '/' . $path, 'source' => $source, ); \OCP\Util::emitHook('\OC\Files\Storage\Shared', 'file_put_contents', $info); @@ -332,7 +324,7 @@ class Shared extends \OC\Files\Storage\Common { WHERE `id` = ?' ); - $result = $query->execute(array($relTargetPath, $this->shareId)); + $result = $query->execute(array($relTargetPath, $this->getShareId())); if ($result) { // update the mount manager with the new paths @@ -358,7 +350,7 @@ class Shared extends \OC\Files\Storage\Common { // if we renamed the mount point we need to adjust the file_target in the // database - if (strlen($sourceMountPoint) >= strlen($path1)) { + if (\OC\Files\Filesystem::normalizePath($sourceMountPoint) === \OC\Files\Filesystem::normalizePath($path1)) { return $this->renameMountPoint($path1, $path2); } @@ -417,7 +409,7 @@ class Shared extends \OC\Files\Storage\Common { } } $info = array( - 'target' => $this->mountPoint . $path, + 'target' => $this->getMountPoint() . $path, 'source' => $source, 'mode' => $mode, ); @@ -438,7 +430,7 @@ class Shared extends \OC\Files\Storage\Common { public function free_space($path) { if ($path == '') { - $path = $this->mountPoint; + $path = $this->getMountPoint(); } $source = $this->getSourcePath($path); if ($source) { @@ -471,11 +463,7 @@ class Shared extends \OC\Files\Storage\Common { foreach ($shares as $share) { \OC\Files\Filesystem::mount('\OC\Files\Storage\Shared', array( - 'shareTarget' => $share['file_target'], - 'shareType' => $share['item_type'], - 'shareId' => $share['id'], - 'fileSource' => $share['file_source'], - 'sharedFrom' => $share['uid_owner'], + 'share' => $share, ), $options['user_dir'] . '/' . $share['file_target']); } @@ -487,7 +475,23 @@ class Shared extends \OC\Files\Storage\Common { * @return string */ public function getMountPoint() { - return ltrim($this->mountPoint, '/'); + return $this->share['file_target']; + } + + /** + * @brief get share type + * @return integer can be single user share (0) group share (1), unique group share name (2) + */ + private function getShareType() { + return $this->share['share_type']; + } + + /** + * @brief get share ID + * @return integer unique share ID + */ + private function getShareId() { + return $this->share['id']; } /** @@ -495,15 +499,15 @@ class Shared extends \OC\Files\Storage\Common { * @return string */ public function getSharedFrom() { - return $this->sharedFrom; + return $this->share['uid_owner']; } /** * @brief return share type, can be "file" or "folder" * @return string */ - public function getShareType() { - return $this->type; + public function getItemType() { + return $this->share['item_type']; } public function hasUpdated($path, $time) { @@ -528,7 +532,7 @@ class Shared extends \OC\Files\Storage\Common { public function getOwner($path) { if ($path == '') { - $path = $this->mountPoint; + $path = $this->getMountPoint(); } $source = $this->getFile($path); if ($source) { @@ -539,7 +543,7 @@ class Shared extends \OC\Files\Storage\Common { public function getETag($path) { if ($path == '') { - $path = $this->mountPoint; + $path = $this->getMountPoint(); } if ($source = $this->getSourcePath($path)) { list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); From bffcbac7a78c8b88b581489cca9bb44795cf81eb Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 14 Apr 2014 12:04:12 +0200 Subject: [PATCH 23/34] allow to rename group share mount points --- apps/files_sharing/lib/sharedstorage.php | 43 ++++++++++++++++++++---- lib/private/share/share.php | 1 + 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 7ce9dd58b4..25e6c0abd2 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -317,14 +317,27 @@ class Shared extends \OC\Files\Storage\Common { $relTargetPath = $this->stripUserFilesPath($targetPath); - // rename mount point - $query = \OC_DB::prepare( - 'Update `*PREFIX*share` - SET `file_target` = ? - WHERE `id` = ?' - ); + // if the user renames a mount point from a group share we need to create a new db entry + // for the unique name + if ($this->getShareType() === \OCP\Share::SHARE_TYPE_GROUP && $this->uniqueNameSet() === false) { + $query = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (`item_type`, `item_source`, `item_target`,' + .' `share_type`, `share_with`, `uid_owner`, `permissions`, `stime`, `file_source`,' + .' `file_target`, `token`, `parent`) VALUES (?,?,?,?,?,?,?,?,?,?,?,?)'); + $arguments = array($this->share['item_type'], $this->share['item_source'], $this->share['item_target'], + 2, \OCP\User::getUser(), $this->share['uid_owner'], $this->share['permissions'], $this->share['stime'], $this->share['file_source'], + $relTargetPath, $this->share['token'], $this->share['id']); - $result = $query->execute(array($relTargetPath, $this->getShareId())); + } else { + // rename mount point + $query = \OC_DB::prepare( + 'Update `*PREFIX*share` + SET `file_target` = ? + WHERE `id` = ?' + ); + $arguments = array($relTargetPath, $this->getShareId()); + } + + $result = $query->execute($arguments); if ($result) { // update the mount manager with the new paths @@ -333,6 +346,7 @@ class Shared extends \OC\Files\Storage\Common { $mount->setMountPoint($targetPath . '/'); $mountManager->addMount($mount); $mountManager->removeMount($sourcePath . '/'); + $this->setUniqueName(); } else { \OCP\Util::writeLog('file sharing', @@ -486,6 +500,21 @@ class Shared extends \OC\Files\Storage\Common { return $this->share['share_type']; } + /** + * @brief does the group share already has a user specific unique name + * @return bool + */ + private function uniqueNameSet() { + return (isset($this->share['unique_name']) && $this->share['unique_name']); + } + + /** + * @brief the share now uses a unique name of this user + */ + private function setUniqueName() { + $this->share['unique_name'] = true; + } + /** * @brief get share ID * @return integer unique share ID diff --git a/lib/private/share/share.php b/lib/private/share/share.php index 7af68d1b25..756a4d173e 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -1139,6 +1139,7 @@ class Share extends \OC\Share\Constants { // Filter out duplicate group shares for users with unique targets 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 $row['share_with'] = $items[$row['parent']]['share_with']; // Remove the parent group share unset($items[$row['parent']]); From aae22b2d6a5be03fca8de68e43305373dc07b3c4 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 14 Apr 2014 15:04:27 +0200 Subject: [PATCH 24/34] update script, create Shared folder and adjust target path for the shares --- apps/files_sharing/appinfo/update.php | 103 +++++++++++--------------- apps/files_sharing/appinfo/version | 2 +- 2 files changed, 44 insertions(+), 61 deletions(-) diff --git a/apps/files_sharing/appinfo/update.php b/apps/files_sharing/appinfo/update.php index ab32108ea2..ebeeae10d2 100644 --- a/apps/files_sharing/appinfo/update.php +++ b/apps/files_sharing/appinfo/update.php @@ -1,73 +1,56 @@ execute(); - $groupShares = array(); - //we need to set up user backends, otherwise creating the shares will fail with "because user does not exist" + $view = new \OC\Files\View('/'); + $users = array(); + $shares = array(); + //we need to set up user backends OC_User::useBackend(new OC_User_Database()); OC_Group::useBackend(new OC_Group_Database()); OC_App::loadApps(array('authentication')); - $rootView = new \OC\Files\View(''); + //we need to set up user backends, otherwise creating the shares will fail with "because user does not exist" while ($row = $result->fetchRow()) { - $meta = $rootView->getFileInfo($$row['source']); - $itemSource = $meta['fileid']; - if ($itemSource != -1) { - $file = $meta; - if ($file['mimetype'] == 'httpd/unix-directory') { - $itemType = 'folder'; - } else { - $itemType = 'file'; - } - if ($row['permissions'] == 0) { - $permissions = OCP\PERMISSION_READ | OCP\PERMISSION_SHARE; - } else { - $permissions = OCP\PERMISSION_READ | OCP\PERMISSION_UPDATE | OCP\PERMISSION_SHARE; - if ($itemType == 'folder') { - $permissions |= OCP\PERMISSION_CREATE; - } - } - $pos = strrpos($row['uid_shared_with'], '@'); - if ($pos !== false && OC_Group::groupExists(substr($row['uid_shared_with'], $pos + 1))) { - $shareType = OCP\Share::SHARE_TYPE_GROUP; - $shareWith = substr($row['uid_shared_with'], 0, $pos); - if (isset($groupShares[$shareWith][$itemSource])) { - continue; - } else { - $groupShares[$shareWith][$itemSource] = true; - } - } else if ($row['uid_shared_with'] == 'public') { - $shareType = OCP\Share::SHARE_TYPE_LINK; - $shareWith = null; - } else { - $shareType = OCP\Share::SHARE_TYPE_USER; - $shareWith = $row['uid_shared_with']; - } - OCP\JSON::checkUserExists($row['uid_owner']); - OC_User::setUserId($row['uid_owner']); - //we need to setup the filesystem for the user, otherwise OC_FileSystem::getRoot will fail and break - OC_Util::setupFS($row['uid_owner']); - try { - OCP\Share::shareItem($itemType, $itemSource, $shareType, $shareWith, $permissions); - } - catch (Exception $e) { - $update_error = true; - OCP\Util::writeLog('files_sharing', - 'Upgrade Routine: Skipping sharing "'.$row['source'].'" to "'.$shareWith - .'" (error is "'.$e->getMessage().'")', - OCP\Util::WARN); - } - OC_Util::tearDownFS(); + //collect all user shares + if ($row['share_type'] === "0" && ($row['item_type'] === 'file' || $row['item_type'] === 'folder')) { + $users[] = $row['share_with']; + $shares[$row['id']] = $row['file_target']; + } else if ($row['share_type'] === "1" && ($row['item_type'] === 'file' || $row['item_type'] === 'folder')) { + //collect all group sharesX + $users = array_merge($users, \OC_group::usersInGroup($row['share_with'])); + $shares[$row['id']] = $row['file_target']; + } else if ($row['share_type'] === "2") { + $shares[$row['id']] = $row['file_target']; } } - OC_User::setUserId(null); - if ($update_error) { - OCP\Util::writeLog('files_sharing', 'There were some problems upgrading the sharing of files', OCP\Util::ERROR); + + $unique_users = array_unique($users); + + if (!empty($unique_users) && !empty($shares)) { + + // create folder Shared for each user + + foreach ($unique_users as $user) { + \OC\Files\Filesystem::initMountPoints($user); + if (!$view->file_exists('/' . $user . '/files/Shared')) { + $view->mkdir('/' . $user . '/files/Shared'); + } + } + + $statement = "UPDATE `*PREFIX*share` SET `file_target` = CASE id "; + //update share table + $ids = implode(',', array_keys($shares)); + foreach ($shares as $id => $target) { + $statement .= "WHEN " . $id . " THEN '/Shared" . $target . "' "; + } + $statement .= ' END WHERE `id` IN (' . $ids . ')'; + + $query = OCP\DB::prepare($statement); + $query->execute(array()); } - // NOTE: Let's drop the table after more testing -// $query = OCP\DB::prepare('DROP TABLE `*PREFIX*sharing`'); -// $query->execute(); + } // clean up oc_share table from files which are no longer exists diff --git a/apps/files_sharing/appinfo/version b/apps/files_sharing/appinfo/version index 8f91d33378..bd73f47072 100644 --- a/apps/files_sharing/appinfo/version +++ b/apps/files_sharing/appinfo/version @@ -1 +1 @@ -0.3.5.6 +0.4 From b712393e72fc22dc9d38f074b2eca848e6439bcf Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 14 Apr 2014 17:08:46 +0200 Subject: [PATCH 25/34] fix etag propagation --- apps/files_sharing/appinfo/app.php | 2 - apps/files_sharing/lib/helper.php | 35 ++++++++++++++ apps/files_sharing/lib/updater.php | 77 +++++++++++++----------------- 3 files changed, 68 insertions(+), 46 deletions(-) diff --git a/apps/files_sharing/appinfo/app.php b/apps/files_sharing/appinfo/app.php index 217bc005fa..0ef3457811 100644 --- a/apps/files_sharing/appinfo/app.php +++ b/apps/files_sharing/appinfo/app.php @@ -17,6 +17,4 @@ OCP\Util::addScript('files_sharing', 'share'); \OC_Hook::connect('OC_Filesystem', 'post_delete', '\OC\Files\Cache\Shared_Updater', 'postDeleteHook'); \OC_Hook::connect('OC_Filesystem', 'delete', '\OC\Files\Cache\Shared_Updater', 'deleteHook'); \OC_Hook::connect('OC_Filesystem', 'post_rename', '\OC\Files\Cache\Shared_Updater', 'renameHook'); -\OC_Hook::connect('OCP\Share', 'post_shared', '\OC\Files\Cache\Shared_Updater', 'shareHook'); -\OC_Hook::connect('OCP\Share', 'pre_unshare', '\OC\Files\Cache\Shared_Updater', 'shareHook'); \OC_Hook::connect('OC_Appconfig', 'post_set_value', '\OCA\Files\Share\Maintainer', 'configChangeHook'); diff --git a/apps/files_sharing/lib/helper.php b/apps/files_sharing/lib/helper.php index b602fe3599..1381c0002d 100644 --- a/apps/files_sharing/lib/helper.php +++ b/apps/files_sharing/lib/helper.php @@ -111,4 +111,39 @@ class Helper { } return true; } + + public static function getSharesFromItem($target) { + $result = array(); + $owner = \OC\Files\Filesystem::getOwner($target); + \OC\Files\Filesystem::initMountPoints($owner); + $info = \OC\Files\Filesystem::getFileInfo($target); + $ownerView = new \OC\Files\View('/'.$owner.'/files'); + if ( $owner != \OCP\User::getUser() ) { + $path = $ownerView->getPath($info['fileid']); + } else { + $path = $target; + } + + + $ids = array(); + while ($path !== '' && $path !== '.' && $path !== '/') { + $info = $ownerView->getFileInfo($path); + $ids[] = $info['fileid']; + $path = dirname($path); + } + + if (!empty($ids)) { + + $idList = array_chunk($ids, 99, true); + + foreach ($idList as $subList) { + $statement = "SELECT `share_with`, `share_type`, `file_target` FROM `*PREFIX*share` WHERE `file_source` IN (" . implode(',', $subList) . ") AND `share_type` IN (0, 1, 2)"; + $query = \OCP\DB::prepare($statement); + $r = $query->execute(); + $result = array_merge($result, $r->fetchAll()); + } + } + + return $result; + } } diff --git a/apps/files_sharing/lib/updater.php b/apps/files_sharing/lib/updater.php index e3a7679292..f7c0a75aee 100644 --- a/apps/files_sharing/lib/updater.php +++ b/apps/files_sharing/lib/updater.php @@ -26,31 +26,48 @@ class Shared_Updater { // shares which can be removed from oc_share after the delete operation was successful static private $toRemove = array(); + /** + * @brief walk up the users file tree and update the etags + * @param string $user + * @param string $path + */ + static private function correctUsersFolder($user, $path) { + // $path points to the mount point which is a virtual folder, so we start with + // the parent + $path = '/files' . dirname($path); + \OC\Files\Filesystem::initMountPoints($user); + $view = new \OC\Files\View('/' . $user); + if ($view->file_exists($path)) { + while ($path !== '/') { + $etag = $view->getETag($path); + $view->putFileInfo($path, array('etag' => $etag)); + $path = dirname($path); + } + } else { + error_log("error!" . 'can not update etags on ' . $path . ' for user ' . $user); + \OCP\Util::writeLog('files_sharing', 'can not update etags on ' . $path . ' for user ' . $user, \OCP\Util::ERROR); + } + } + /** * Correct the parent folders' ETags for all users shared the file at $target * * @param string $target */ static public function correctFolders($target) { - $uid = \OCP\User::getUser(); - $uidOwner = \OC\Files\Filesystem::getOwner($target); - $info = \OC\Files\Filesystem::getFileInfo($target); - $checkedUser = array($uidOwner); // Correct Shared folders of other users shared with - $users = \OCP\Share::getUsersItemShared('file', $info['fileid'], $uidOwner, true); - if (!empty($users)) { - while (!empty($users)) { - $reshareUsers = array(); + $shares = \OCA\Files_Sharing\Helper::getSharesFromItem($target); + + foreach ($shares as $share) { + if ((int)$share['share_type'] === \OCP\Share::SHARE_TYPE_USER) { + self::correctUsersFolder($share['share_with'], $share['file_target']); + } elseif ((int)$share['share_type'] === \OCP\Share::SHARE_TYPE_GROUP) { + $users = \OC_Group::usersInGroup($share['share_with']); foreach ($users as $user) { - if ( !in_array($user, $checkedUser) ) { - $etag = \OC\Files\Filesystem::getETag(''); - \OCP\Config::setUserValue($user, 'files_sharing', 'etag', $etag); - // Look for reshares - $reshareUsers = array_merge($reshareUsers, \OCP\Share::getUsersItemShared('file', $info['fileid'], $user, true)); - $checkedUser[] = $user; - } + self::correctUsersFolder($user, $share['file_target']); } - $users = $reshareUsers; + } else { //unique name for group share + self::correctUsersFolder($share['share_with'], $share['file_target']); } } } @@ -107,34 +124,6 @@ class Shared_Updater { self::removeShare($params['path']); } - /** - * @param array $params - */ - static public function shareHook($params) { - if ($params['itemType'] === 'file' || $params['itemType'] === 'folder') { - if (isset($params['uidOwner'])) { - $uidOwner = $params['uidOwner']; - } else { - $uidOwner = \OCP\User::getUser(); - } - $users = \OCP\Share::getUsersItemShared($params['itemType'], $params['fileSource'], $uidOwner, true, false); - if (!empty($users)) { - while (!empty($users)) { - $reshareUsers = array(); - foreach ($users as $user) { - if ($user !== $uidOwner) { - $etag = \OC\Files\Filesystem::getETag(''); - \OCP\Config::setUserValue($user, 'files_sharing', 'etag', $etag); - // Look for reshares - $reshareUsers = array_merge($reshareUsers, \OCP\Share::getUsersItemShared('file', $params['fileSource'], $user, true)); - } - } - $users = $reshareUsers; - } - } - } - } - /** * clean up oc_share table from files which are no longer exists * From 652d417a585ede1564456c446577aa1752253ccd Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 15 Apr 2014 11:19:31 +0200 Subject: [PATCH 26/34] we don't allow to share a folder if it contains a share mount point --- apps/files_sharing/lib/cache.php | 7 +-- apps/files_sharing/lib/sharedstorage.php | 5 ++ apps/files_sharing/tests/api.php | 60 ++++++++++++++++++++++++ apps/files_sharing/tests/cache.php | 6 +-- lib/private/files/view.php | 3 +- lib/private/share/share.php | 16 +++++++ 6 files changed, 90 insertions(+), 7 deletions(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index 7e44847e40..3d9fbcf4de 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -411,7 +411,7 @@ class Shared_Cache extends Cache { } /** - * get the path of a file on this storage by it's id + * get the path of a file on this storage relative to the mount point by it's id * * @param int $id * @param string $pathEnd (optional) used internally for recursive calls @@ -419,8 +419,9 @@ class Shared_Cache extends Cache { */ public function getPathById($id, $pathEnd = '') { // direct shares are easy - if ($path = $this->getShareById($id)) { - return $path . $pathEnd; + $path = $this->getShareById($id); + if (is_string($path)) { + return ltrim($pathEnd, '/'); } else { // if the item is a direct share we try and get the path of the parent and append the name of the item to it list($parent, $name) = $this->getParentInfo($id); diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 25e6c0abd2..eedd279bf2 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -347,6 +347,7 @@ class Shared extends \OC\Files\Storage\Common { $mountManager->addMount($mount); $mountManager->removeMount($sourcePath . '/'); $this->setUniqueName(); + $this->setMountPoint($relTargetPath); } else { \OCP\Util::writeLog('file sharing', @@ -500,6 +501,10 @@ class Shared extends \OC\Files\Storage\Common { return $this->share['share_type']; } + private function setMountPoint($path) { + $this->share['file_target'] = $path; + } + /** * @brief does the group share already has a user specific unique name * @return bool diff --git a/apps/files_sharing/tests/api.php b/apps/files_sharing/tests/api.php index 6354d1099b..5975eb9588 100644 --- a/apps/files_sharing/tests/api.php +++ b/apps/files_sharing/tests/api.php @@ -866,6 +866,66 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { $this->assertTrue($result3->succeeded()); + // cleanup + \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER1); + + $result = \OCP\Share::unshare('folder', $fileInfo1['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2); + + $this->assertTrue($result); + + + + } + + /** + * @brief share a folder which contains a share mount point, should be forbidden + */ + public function testShareFolderWithAMountPoint() { + // user 1 shares a folder with user2 + \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER1); + + $fileInfo = $this->view->getFileInfo($this->folder); + + $result = \OCP\Share::shareItem('folder', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2, 31); + + $this->assertTrue($result); + + // user2 shares a file from the folder as link + \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2); + + $view = new \OC\Files\View('/' . \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2 . '/files'); + $view->mkdir("localDir"); + + // move mount point to the folder "localDir" + $result = $view->rename($this->folder, 'localDir/'.$this->folder); + $this->assertTrue($result !== false); + + // try to share "localDir" + $fileInfo2 = $view->getFileInfo('localDir'); + + $this->assertTrue($fileInfo2 instanceof \OC\Files\FileInfo); + + try { + $result2 = \OCP\Share::shareItem('folder', $fileInfo2['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER3, 31); + } catch (\Exception $e) { + $result2 = false; + } + + $this->assertFalse($result2); + + //cleanup + + $result = $view->rename('localDir/' . $this->folder, $this->folder); + $this->assertTrue($result !== false); + $view->unlink('localDir'); + + \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER1); + + \OCP\Share::unshare('folder', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2); } } diff --git a/apps/files_sharing/tests/cache.php b/apps/files_sharing/tests/cache.php index 7a52f403d8..b8ebeab3c3 100644 --- a/apps/files_sharing/tests/cache.php +++ b/apps/files_sharing/tests/cache.php @@ -255,7 +255,7 @@ class Test_Files_Sharing_Cache extends Test_Files_Sharing_Base { */ $sharedCache = $sharedStorage->getCache(); - $this->assertEquals('test.txt', $sharedCache->getPathById($info->getId())); + $this->assertEquals('', $sharedCache->getPathById($info->getId())); } public function testGetPathByIdShareSubFolder() { @@ -276,7 +276,7 @@ class Test_Files_Sharing_Cache extends Test_Files_Sharing_Base { */ $sharedCache = $sharedStorage->getCache(); - $this->assertEquals('foo', $sharedCache->getPathById($folderInfo->getId())); - $this->assertEquals('foo/bar/test.txt', $sharedCache->getPathById($fileInfo->getId())); + $this->assertEquals('', $sharedCache->getPathById($folderInfo->getId())); + $this->assertEquals('bar/test.txt', $sharedCache->getPathById($fileInfo->getId())); } } diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 3f73632be1..a61d58aaf2 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -1168,7 +1168,8 @@ class View { * @var \OC\Files\Mount\Mount $mount */ $cache = $mount->getStorage()->getCache(); - if ($internalPath = $cache->getPathById($id)) { + $internalPath = $cache->getPathById($id); + if (is_string($internalPath)) { $fullPath = $mount->getMountPoint() . $internalPath; if (!is_null($path = $this->getRelativePath($fullPath))) { return $path; diff --git a/lib/private/share/share.php b/lib/private/share/share.php index 756a4d173e..c07dd04b29 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -489,6 +489,7 @@ class Share extends \OC\Share\Constants { $itemSourceName = $itemSource; } + // verify that the file exists before we try to share it if ($itemType === 'file' or $itemType === 'folder') { $path = \OC\Files\Filesystem::getPath($itemSource); @@ -499,6 +500,21 @@ class Share extends \OC\Share\Constants { } } + //verify that we don't share a folder which already contains a share mount point + if ($itemType === 'folder') { + $path = '/' . $uidOwner . '/files' . \OC\Files\Filesystem::getPath($itemSource) . '/'; + $mountManager = \OC\Files\Filesystem::getMountManager(); + $mounts = $mountManager->getAll(); + foreach ($mounts as $mountPoint => $mount) { + if ($mount->getStorage() instanceof \OC\Files\Storage\Shared && strpos($mountPoint, $path) === 0) { + $message = 'Sharing "' . $itemSourceName . '" failed, because it contains files shared with you!'; + \OC_Log::write('OCP\Share', $message, \OC_Log::ERROR); + throw new \Exception($message); + } + + } + } + // Verify share type and sharing conditions are met if ($shareType === self::SHARE_TYPE_USER) { if ($shareWith == $uidOwner) { From d468cdacf27acf1de78a7b2f07d21d1851aa8f39 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 15 Apr 2014 15:48:54 +0200 Subject: [PATCH 27/34] add unit tests which got lost during rebase --- apps/files_sharing/tests/api.php | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/apps/files_sharing/tests/api.php b/apps/files_sharing/tests/api.php index 5975eb9588..b2f05d10ac 100644 --- a/apps/files_sharing/tests/api.php +++ b/apps/files_sharing/tests/api.php @@ -928,4 +928,27 @@ class Test_Files_Sharing_Api extends Test_Files_Sharing_Base { \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2); } + /** + * @expectedException \Exception + */ + public function testShareNonExisting() { + \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER1); + + $id = PHP_INT_MAX - 1; + \OCP\Share::shareItem('file', $id, \OCP\Share::SHARE_TYPE_LINK, \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2, 31); + } + + /** + * @expectedException \Exception + */ + public function testShareNotOwner() { + \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2); + \OC\Files\Filesystem::file_put_contents('foo.txt', 'bar'); + $info = \OC\Files\Filesystem::getFileInfo('foo.txt'); + + \Test_Files_Sharing_Api::loginHelper(\Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER1); + + \OCP\Share::shareItem('file', $info->getId(), \OCP\Share::SHARE_TYPE_LINK, \Test_Files_Sharing_Api::TEST_FILES_SHARING_API_USER2, 31); + } + } From fb88aba8f4927b3175df34a2a499978a3b4c1b6b Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 15 Apr 2014 20:18:04 +0200 Subject: [PATCH 28/34] some fixes to make the gallery work, this made it necessary to adjust some tests and the encryption code --- apps/files_encryption/hooks/hooks.php | 127 +++----------------------- apps/files_encryption/lib/util.php | 101 ++------------------ apps/files_sharing/lib/cache.php | 5 +- apps/files_sharing/tests/cache.php | 18 ++-- lib/private/share/share.php | 1 + 5 files changed, 33 insertions(+), 219 deletions(-) diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php index 7a8b54e2d7..5f0494e62c 100644 --- a/apps/files_encryption/hooks/hooks.php +++ b/apps/files_encryption/hooks/hooks.php @@ -302,25 +302,6 @@ class Hooks { */ public static function postShared($params) { - // NOTE: $params has keys: - // [itemType] => file - // itemSource -> int, filecache file ID - // [parent] => - // [itemTarget] => /13 - // shareWith -> string, uid of user being shared to - // fileTarget -> path of file being shared - // uidOwner -> owner of the original file being shared - // [shareType] => 0 - // [shareWith] => test1 - // [uidOwner] => admin - // [permissions] => 17 - // [fileSource] => 13 - // [fileTarget] => /test8 - // [id] => 10 - // [token] => - // [run] => whether emitting script should continue to run - // TODO: Should other kinds of item be encrypted too? - if (\OCP\App::isEnabled('files_encryption') === false) { return true; } @@ -331,68 +312,22 @@ class Hooks { $session = new \OCA\Encryption\Session($view); $userId = \OCP\User::getUser(); $util = new Util($view, $userId); - $path = $util->fileIdToPath($params['itemSource']); - - $share = $util->getParentFromShare($params['id']); - //if parent is set, then this is a re-share action - if ($share['parent'] !== null) { - - // get the parent from current share - $parent = $util->getShareParent($params['parent']); - - // if parent has the same type than the child it is a 1:1 share - if ($parent['item_type'] === $params['itemType']) { - $path = $parent['file_target']; - } else { - - // NOTE: parent is folder but shared was a file! - // we try to rebuild the missing path - // some examples we face here - // user1 share folder1 with user2 folder1 has - // the following structure - // /folder1/subfolder1/subsubfolder1/somefile.txt - // user2 re-share subfolder2 with user3 - // user3 re-share somefile.txt user4 - // so our path should be - // /Shared/subfolder1/subsubfolder1/somefile.txt - // while user3 is sharing - - if ($params['itemType'] === 'file') { - // get target path - $targetPath = $util->fileIdToPath($params['fileSource']); - $targetPathSplit = array_reverse(explode('/', $targetPath)); - - // init values - $path = ''; - $sharedPart = ltrim($parent['file_target'], '/'); - - // rebuild path - foreach ($targetPathSplit as $pathPart) { - if ($pathPart !== $sharedPart) { - $path = '/' . $pathPart . $path; - } else { - break; - } - } - $path = $parent['file_target'] . $path; - } else { - $path = $parent['file_target'] . $params['fileTarget']; - } - } - } + $path = \OC\Files\Filesystem::getPath($params['fileSource']); $sharingEnabled = \OCP\Share::isEnabled(); // get the path including mount point only if not a shared folder list($storage, ) = \OC\Files\Filesystem::resolvePath('/' . $userId . '/files' . $path); - if (!($storage instanceof \OC\Files\Storage\Shared)) { - // get path including the the storage mount point - $path = $util->getPathWithMountPoint($params['itemSource']); + + if (!($storage instanceof \OC\Files\Storage\Local)) { + $mountPoint = 'files' . $storage->getMountPoint(); + } else { + $mountPoint = ''; } // if a folder was shared, get a list of all (sub-)folders if ($params['itemType'] === 'folder') { - $allFiles = $util->getAllFiles($path); + $allFiles = $util->getAllFiles($path, $mountPoint); } else { $allFiles = array($path); } @@ -409,13 +344,6 @@ class Hooks { */ public static function postUnshare($params) { - // NOTE: $params has keys: - // [itemType] => file - // [itemSource] => 13 - // [shareType] => 0 - // [shareWith] => test1 - // [itemParent] => - if (\OCP\App::isEnabled('files_encryption') === false) { return true; } @@ -425,34 +353,7 @@ class Hooks { $view = new \OC_FilesystemView('/'); $userId = \OCP\User::getUser(); $util = new Util($view, $userId); - $path = $util->fileIdToPath($params['itemSource']); - - // check if this is a re-share - if ($params['itemParent']) { - - // get the parent from current share - $parent = $util->getShareParent($params['itemParent']); - - // get target path - $targetPath = $util->fileIdToPath($params['itemSource']); - $targetPathSplit = array_reverse(explode('/', $targetPath)); - - // init values - $path = ''; - $sharedPart = ltrim($parent['file_target'], '/'); - - // rebuild path - foreach ($targetPathSplit as $pathPart) { - if ($pathPart !== $sharedPart) { - $path = '/' . $pathPart . $path; - } else { - break; - } - } - - // prefix path with Shared - $path = $parent['file_target'] . $path; - } + $path = \OC\Files\Filesystem::getPath($params['fileSource']); // for group shares get a list of the group members if ($params['shareType'] === \OCP\Share::SHARE_TYPE_GROUP) { @@ -466,15 +367,17 @@ class Hooks { } // get the path including mount point only if not a shared folder - list($storage, ) = \OC\Files\Filesystem::resolvePath($path); - if (!($storage instanceof \OC\Files\Storage\Shared)) { - // get path including the the storage mount point - //$path = $util->getPathWithMountPoint($params['itemSource']); + list($storage, ) = \OC\Files\Filesystem::resolvePath('/' . $userId . '/files' . $path); + + if (!($storage instanceof \OC\Files\Storage\Local)) { + $mountPoint = 'files' . $storage->getMountPoint(); + } else { + $mountPoint = ''; } // if we unshare a folder we need a list of all (sub-)files if ($params['itemType'] === 'folder') { - $allFiles = $util->getAllFiles($path); + $allFiles = $util->getAllFiles($path, $mountPoint); } else { $allFiles = array($path); } diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php index 4be4ab9565..6372ab31b6 100644 --- a/apps/files_encryption/lib/util.php +++ b/apps/files_encryption/lib/util.php @@ -969,33 +969,6 @@ class Util { } - /** - * @brief get path of a file. - * @param int $fileId id of the file - * @return string path of the file - */ - public static function fileIdToPath($fileId) { - - $sql = 'SELECT `path` FROM `*PREFIX*filecache` WHERE `fileid` = ?'; - - $query = \OCP\DB::prepare($sql); - - $result = $query->execute(array($fileId)); - - $path = false; - if (\OCP\DB::isError($result)) { - \OCP\Util::writeLog('Encryption library', \OC_DB::getErrorMessage($result), \OCP\Util::ERROR); - } else { - $row = $result->fetchRow(); - if ($row) { - $path = substr($row['path'], strlen('files')); - } - } - - return $path; - - } - /** * @brief Filter an array of UIDs to return only ones ready for sharing * @param array $unfilteredUsers users to be checked for sharing readiness @@ -1398,7 +1371,7 @@ class Util { * @param string $dir relative to the users files folder * @return array with list of files relative to the users files folder */ - public function getAllFiles($dir) { + public function getAllFiles($dir, $mountPoint = '') { $result = array(); $dirList = array($dir); @@ -1408,10 +1381,13 @@ class Util { $this->userFilesDir . '/' . $dir)); foreach ($content as $c) { + // getDirectoryContent() returns the paths relative to the mount points, so we need + // to re-construct the complete path + $path = ($mountPoint !== '') ? $mountPoint . '/' . $c['path'] : $c['path']; if ($c['type'] === 'dir') { - $dirList[] = substr($c['path'], strlen("files")); + $dirList[] = substr($path, strlen("files")); } else { - $result[] = substr($c['path'], strlen("files")); + $result[] = substr($path, strlen("files")); } } @@ -1420,54 +1396,6 @@ class Util { return $result; } - /** - * @brief get shares parent. - * @param int $id of the current share - * @return array of the parent - */ - public static function getShareParent($id) { - - $sql = 'SELECT `file_target`, `item_type` FROM `*PREFIX*share` WHERE `id` = ?'; - - $query = \OCP\DB::prepare($sql); - - $result = $query->execute(array($id)); - - $row = array(); - if (\OCP\DB::isError($result)) { - \OCP\Util::writeLog('Encryption library', \OC_DB::getErrorMessage($result), \OCP\Util::ERROR); - } else { - $row = $result->fetchRow(); - } - - return $row; - - } - - /** - * @brief get shares parent. - * @param int $id of the current share - * @return array of the parent - */ - public static function getParentFromShare($id) { - - $sql = 'SELECT `parent` FROM `*PREFIX*share` WHERE `id` = ?'; - - $query = \OCP\DB::prepare($sql); - - $result = $query->execute(array($id)); - - $row = array(); - if (\OCP\DB::isError($result)) { - \OCP\Util::writeLog('Encryption library', \OC_DB::getErrorMessage($result), \OCP\Util::ERROR); - } else { - $row = $result->fetchRow(); - } - - return $row; - - } - /** * @brief get owner of the shared files. * @param $id @@ -1709,23 +1637,6 @@ class Util { $this->recoverAllFiles('/', $privateKey); } - /** - * Get the path including the storage mount point - * @param int $id - * @return string the path including the mount point like AmazonS3/folder/file.txt - */ - public function getPathWithMountPoint($id) { - list($storage, $internalPath) = \OC\Files\Cache\Cache::getById($id); - $mount = \OC\Files\Filesystem::getMountByStorageId($storage); - $mountPoint = $mount[0]->getMountPoint(); - $path = \OC\Files\Filesystem::normalizePath($mountPoint . '/' . $internalPath); - - // reformat the path to be relative e.g. /user/files/folder becomes /folder/ - $relativePath = \OCA\Encryption\Helper::stripUserFilesPath($path); - - return $relativePath; - } - /** * @brief check if the file is stored on a system wide mount point * @param $path relative to /data/user with leading '/' diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index 3d9fbcf4de..4b473c6057 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -141,15 +141,14 @@ class Shared_Cache extends Cache { $folder = ''; } - $dir = 'files' . $this->storage->getMountPoint(); - $dir .= ($folder !== '') ? '/' . $folder : ''; + $dir = ($folder !== '') ? $folder . '/' : ''; $cache = $this->getSourceCache($folder); if ($cache) { $parent = $this->storage->getFile($folder); $sourceFolderContent = $cache->getFolderContents($this->files[$folder]); foreach ($sourceFolderContent as $key => $c) { - $sourceFolderContent[$key]['path'] = $dir . '/' . $c['name']; + $sourceFolderContent[$key]['path'] = $dir . $c['name']; $sourceFolderContent[$key]['uid_owner'] = $parent['uid_owner']; $sourceFolderContent[$key]['displayname_owner'] = $parent['uid_owner']; } diff --git a/apps/files_sharing/tests/cache.php b/apps/files_sharing/tests/cache.php index b8ebeab3c3..1af73c558d 100644 --- a/apps/files_sharing/tests/cache.php +++ b/apps/files_sharing/tests/cache.php @@ -100,15 +100,15 @@ class Test_Files_Sharing_Cache extends Test_Files_Sharing_Base { $check = array( array( 'name' => 'bar.txt', - 'path' => 'files/shareddir/bar.txt' + 'path' => 'bar.txt' ), array( 'name' => 'another too.txt', - 'path' => 'files/shareddir/subdir/another too.txt' + 'path' => 'subdir/another too.txt' ), array( 'name' => 'another.txt', - 'path' => 'files/shareddir/subdir/another.txt' + 'path' => 'subdir/another.txt' ), ); $this->verifyFiles($check, $results); @@ -151,17 +151,17 @@ class Test_Files_Sharing_Cache extends Test_Files_Sharing_Base { array( array( 'name' => 'bar.txt', - 'path' => 'files/shareddir/bar.txt', + 'path' => 'bar.txt', 'mimetype' => 'text/plain', ), array( 'name' => 'emptydir', - 'path' => 'files/shareddir/emptydir', + 'path' => 'emptydir', 'mimetype' => 'httpd/unix-directory', ), array( 'name' => 'subdir', - 'path' => 'files/shareddir/subdir', + 'path' => 'subdir', 'mimetype' => 'httpd/unix-directory', ), ), @@ -185,17 +185,17 @@ class Test_Files_Sharing_Cache extends Test_Files_Sharing_Base { array( array( 'name' => 'another too.txt', - 'path' => 'files/subdir/another too.txt', + 'path' => 'another too.txt', 'mimetype' => 'text/plain', ), array( 'name' => 'another.txt', - 'path' => 'files/subdir/another.txt', + 'path' => 'another.txt', 'mimetype' => 'text/plain', ), array( 'name' => 'not a text file.xml', - 'path' => 'files/subdir/not a text file.xml', + 'path' => 'not a text file.xml', 'mimetype' => 'application/xml', ), ), diff --git a/lib/private/share/share.php b/lib/private/share/share.php index c07dd04b29..4e3e261baf 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -896,6 +896,7 @@ class Share extends \OC\Share\Constants { $hookParams = array( 'itemType' => $item['item_type'], 'itemSource' => $item['item_source'], + 'fileSource' => $item['file_source'], 'shareType' => $item['share_type'], 'shareWith' => $item['share_with'], 'itemParent' => $item['parent'], From 2049bedcaf670ef2394ce8b19e0d3f2174513b53 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 15 Apr 2014 16:36:05 +0200 Subject: [PATCH 29/34] Dont throw an error when a storage isn't found for shared cache --- apps/files_sharing/lib/cache.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index 4b473c6057..4a2f0ff08b 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -53,9 +53,9 @@ class Shared_Cache extends Cache { $source = \OC_Share_Backend_File::getSource($target, $this->storage->getMountPoint(), $this->storage->getItemType()); if (isset($source['path']) && isset($source['fileOwner'])) { \OC\Files\Filesystem::initMountPoints($source['fileOwner']); - $mount = \OC\Files\Filesystem::getMountByNumericId($source['storage']); - if (is_array($mount)) { - $fullPath = $mount[key($mount)]->getMountPoint() . $source['path']; + $mounts = \OC\Files\Filesystem::getMountByNumericId($source['storage']); + if (is_array($mounts) and count($mounts)) { + $fullPath = $mounts[0]->getMountPoint() . $source['path']; list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($fullPath); if ($storage) { $this->files[$target] = $internalPath; From dd1e47b3b896e2ee59faf5f423bb911c5d2c2548 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 15 Apr 2014 20:26:04 +0200 Subject: [PATCH 30/34] typos, use, unused and return fixed --- apps/files_sharing/appinfo/update.php | 4 +++- apps/files_sharing/lib/helper.php | 7 +++---- apps/files_sharing/lib/sharedstorage.php | 5 +++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/apps/files_sharing/appinfo/update.php b/apps/files_sharing/appinfo/update.php index ebeeae10d2..c79a2291e9 100644 --- a/apps/files_sharing/appinfo/update.php +++ b/apps/files_sharing/appinfo/update.php @@ -18,7 +18,7 @@ if (version_compare($installedVersion, '0.4', '<')) { $users[] = $row['share_with']; $shares[$row['id']] = $row['file_target']; } else if ($row['share_type'] === "1" && ($row['item_type'] === 'file' || $row['item_type'] === 'folder')) { - //collect all group sharesX + //collect all group shares $users = array_merge($users, \OC_group::usersInGroup($row['share_with'])); $shares[$row['id']] = $row['file_target']; } else if ($row['share_type'] === "2") { @@ -48,7 +48,9 @@ if (version_compare($installedVersion, '0.4', '<')) { $statement .= ' END WHERE `id` IN (' . $ids . ')'; $query = OCP\DB::prepare($statement); + $query->execute(array()); + } } diff --git a/apps/files_sharing/lib/helper.php b/apps/files_sharing/lib/helper.php index 1381c0002d..e2780e9893 100644 --- a/apps/files_sharing/lib/helper.php +++ b/apps/files_sharing/lib/helper.php @@ -2,6 +2,9 @@ namespace OCA\Files_Sharing; +use OC_Config; +use PasswordHash; + class Helper { /** @@ -26,9 +29,6 @@ class Helper { exit; } - $type = $linkItem['item_type']; - $fileSource = $linkItem['file_source']; - $shareOwner = $linkItem['uid_owner']; $rootLinkItem = \OCP\Share::resolveReShare($linkItem); $path = null; if (isset($rootLinkItem['uid_owner'])) { @@ -61,7 +61,6 @@ class Helper { } $basePath = $path; - $rootName = basename($path); if ($relativePath !== null && \OC\Files\Filesystem::isReadable($basePath . $relativePath)) { $path .= \OC\Files\Filesystem::normalizePath($relativePath); diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index eedd279bf2..b8a799f720 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -299,14 +299,14 @@ class Shared extends \OC\Files\Storage\Common { } /** - * @brief rename a shared foder/file + * @brief rename a shared folder/file * @param string $sourcePath * @param string $targetPath * @return bool */ private function renameMountPoint($sourcePath, $targetPath) { - // it shoulbn't be possible to move a Shared storage into another one + // it shouldn't be possible to move a Shared storage into another one list($targetStorage, ) = \OC\Files\Filesystem::resolvePath($targetPath); if ($targetStorage instanceof \OC\Files\Storage\Shared) { \OCP\Util::writeLog('file sharing', @@ -452,6 +452,7 @@ class Shared extends \OC\Files\Storage\Common { list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source); return $storage->free_space($internalPath); } + return \OC\Files\SPACE_UNKNOWN; } public function getLocalFile($path) { From 93469ca46865d02d33710a2f70f7f6092c8f5c58 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 16 Apr 2014 16:41:23 +0200 Subject: [PATCH 31/34] make it possible to move files out of a shared mount point --- apps/files_sharing/lib/helper.php | 31 ++++++++++++++++++++++++ apps/files_sharing/lib/sharedstorage.php | 27 +++++++++++---------- lib/private/files/view.php | 8 +++--- 3 files changed, 49 insertions(+), 17 deletions(-) diff --git a/apps/files_sharing/lib/helper.php b/apps/files_sharing/lib/helper.php index e2780e9893..cc1f7d9ffd 100644 --- a/apps/files_sharing/lib/helper.php +++ b/apps/files_sharing/lib/helper.php @@ -145,4 +145,35 @@ class Helper { return $result; } + + public static function getUidAndFilename($filename) { + $uid = \OC\Files\Filesystem::getOwner($filename); + \OC\Files\Filesystem::initMountPoints($uid); + if ( $uid != \OCP\User::getUser() ) { + $info = \OC\Files\Filesystem::getFileInfo($filename); + $ownerView = new \OC\Files\View('/'.$uid.'/files'); + $filename = $ownerView->getPath($info['fileid']); + } + return array($uid, $filename); + } + + /** + * @brief Format a path to be relative to the /user/files/ directory + * @param string $path the absolute path + * @return string e.g. turns '/admin/files/test.txt' into 'test.txt' + */ + public static function stripUserFilesPath($path) { + $trimmed = ltrim($path, '/'); + $split = explode('/', $trimmed); + + // it is not a file relative to data/user/files + if (count($split) < 3 || $split[1] !== 'files') { + return false; + } + + $sliced = array_slice($split, 2); + $relPath = implode('/', $sliced); + + return $relPath; + } } diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index b8a799f720..5e478d5ead 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -362,6 +362,9 @@ class Shared extends \OC\Files\Storage\Common { public function rename($path1, $path2) { $sourceMountPoint = \OC\Files\Filesystem::getMountPoint($path1); + $targetMountPoint = \OC\Files\Filesystem::getMountPoint($path2); + $relPath1 = \OCA\Files_Sharing\Helper::stripUserFilesPath($path1); + $relPath2 = \OCA\Files_Sharing\Helper::stripUserFilesPath($path2); // if we renamed the mount point we need to adjust the file_target in the // database @@ -369,21 +372,19 @@ class Shared extends \OC\Files\Storage\Common { return $this->renameMountPoint($path1, $path2); } - // Renaming/moving is only allowed within shared folders - $oldSource = $this->getSourcePath($path1); - if ($oldSource) { - $newSource = $this->getSourcePath(dirname($path2)) . '/' . basename($path2); - // Within the same folder, we only need UPDATE permissions - if (dirname($path1) == dirname($path2) and $this->isUpdatable($path1)) { - list($storage, $oldInternalPath) = \OC\Files\Filesystem::resolvePath($oldSource); - list(, $newInternalPath) = \OC\Files\Filesystem::resolvePath($newSource); - return $storage->rename($oldInternalPath, $newInternalPath); + + if ( // Within the same mount point, we only need UPDATE permissions + ($sourceMountPoint === $targetMountPoint && $this->isUpdatable($sourceMountPoint)) || // otherwise DELETE and CREATE permissions required - } elseif ($this->isDeletable($path1) && $this->isCreatable(dirname($path2))) { - $rootView = new \OC\Files\View(''); - return $rootView->rename($oldSource, $newSource); - } + ($this->isDeletable($path1) && $this->isCreatable(dirname($path2)))) { + + list($user1, $path1) = \OCA\Files_Sharing\Helper::getUidAndFilename($relPath1); + $targetFilename = basename($relPath2); + list($user2, $path2) = \OCA\Files_Sharing\Helper::getUidAndFilename(dirname($relPath2)); + $rootView = new \OC\Files\View(''); + return $rootView->rename('/' . $user1 . '/files/' . $path1, '/' . $user2 . '/files/' . $path2 . '/' . $targetFilename); } + return false; } diff --git a/lib/private/files/view.php b/lib/private/files/view.php index a61d58aaf2..58dfc73dcf 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -410,16 +410,16 @@ class View { // if source and target are on the same storage we can call the rename operation from the // storage. If it is a "Shared" file/folder we call always the rename operation of the // shared storage to handle mount point renaming, etc correctly - if ($mp1 == $mp2) { + if ($storage1 instanceof \OC\Files\Storage\Shared) { if ($storage1) { - $result = $storage1->rename($internalPath1, $internalPath2); + $result = $storage1->rename($absolutePath1, $absolutePath2); \OC_FileProxy::runPostProxies('rename', $absolutePath1, $absolutePath2); } else { $result = false; } - } elseif ($storage1 instanceof \OC\Files\Storage\Shared) { + } elseif ($mp1 == $mp2) { if ($storage1) { - $result = $storage1->rename($absolutePath1, $absolutePath2); + $result = $storage1->rename($internalPath1, $internalPath2); \OC_FileProxy::runPostProxies('rename', $absolutePath1, $absolutePath2); } else { $result = false; From b102222fed33245c6da8a39c28f0d0a570d0dbea Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 16 Apr 2014 17:43:02 +0200 Subject: [PATCH 32/34] split-up the update script and add unit tests for it --- apps/files_sharing/appinfo/update.php | 62 +++++++++++++++++---------- apps/files_sharing/tests/updater.php | 55 ++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 23 deletions(-) diff --git a/apps/files_sharing/appinfo/update.php b/apps/files_sharing/appinfo/update.php index c79a2291e9..bc8cda4231 100644 --- a/apps/files_sharing/appinfo/update.php +++ b/apps/files_sharing/appinfo/update.php @@ -2,6 +2,21 @@ $installedVersion = OCP\Config::getAppValue('files_sharing', 'installed_version'); if (version_compare($installedVersion, '0.4', '<')) { + removeSharedFolder(); +} + +// clean up oc_share table from files which are no longer exists +if (version_compare($installedVersion, '0.3.5.6', '<')) { + \OC\Files\Cache\Shared_Updater::fixBrokenSharesOnAppUpdate(); +} + + +/** + * update script for the removal of the logical "Shared" folder, we create physical "Shared" folder and + * update the users file_target so that it doesn't make any difference for the user + * @note parameters are just for testing, please ignore them + */ +function removeSharedFolder($mkdirs = true, $chunkSize = 99) { $query = OCP\DB::prepare('SELECT * FROM `*PREFIX*share`'); $result = $query->execute(); $view = new \OC\Files\View('/'); @@ -14,14 +29,14 @@ if (version_compare($installedVersion, '0.4', '<')) { //we need to set up user backends, otherwise creating the shares will fail with "because user does not exist" while ($row = $result->fetchRow()) { //collect all user shares - if ($row['share_type'] === "0" && ($row['item_type'] === 'file' || $row['item_type'] === 'folder')) { + if ((int)$row['share_type'] === 0 && ($row['item_type'] === 'file' || $row['item_type'] === 'folder')) { $users[] = $row['share_with']; $shares[$row['id']] = $row['file_target']; - } else if ($row['share_type'] === "1" && ($row['item_type'] === 'file' || $row['item_type'] === 'folder')) { + } else if ((int)$row['share_type'] === 1 && ($row['item_type'] === 'file' || $row['item_type'] === 'folder')) { //collect all group shares $users = array_merge($users, \OC_group::usersInGroup($row['share_with'])); $shares[$row['id']] = $row['file_target']; - } else if ($row['share_type'] === "2") { + } else if ((int)$row['share_type'] === 2) { $shares[$row['id']] = $row['file_target']; } } @@ -32,30 +47,31 @@ if (version_compare($installedVersion, '0.4', '<')) { // create folder Shared for each user - foreach ($unique_users as $user) { - \OC\Files\Filesystem::initMountPoints($user); - if (!$view->file_exists('/' . $user . '/files/Shared')) { - $view->mkdir('/' . $user . '/files/Shared'); + if ($mkdirs) { + foreach ($unique_users as $user) { + \OC\Files\Filesystem::initMountPoints($user); + if (!$view->file_exists('/' . $user . '/files/Shared')) { + $view->mkdir('/' . $user . '/files/Shared'); + } } } - $statement = "UPDATE `*PREFIX*share` SET `file_target` = CASE id "; - //update share table - $ids = implode(',', array_keys($shares)); - foreach ($shares as $id => $target) { - $statement .= "WHEN " . $id . " THEN '/Shared" . $target . "' "; + $chunkedShareList = array_chunk($shares, $chunkSize, true); + + foreach ($chunkedShareList as $subList) { + + $statement = "UPDATE `*PREFIX*share` SET `file_target` = CASE `id` "; + //update share table + $ids = implode(',', array_keys($subList)); + foreach ($subList as $id => $target) { + $statement .= "WHEN " . $id . " THEN '/Shared" . $target . "' "; + } + $statement .= ' END WHERE `id` IN (' . $ids . ')'; + + $query = OCP\DB::prepare($statement); + + $query->execute(array()); } - $statement .= ' END WHERE `id` IN (' . $ids . ')'; - - $query = OCP\DB::prepare($statement); - - $query->execute(array()); } - -} - -// clean up oc_share table from files which are no longer exists -if (version_compare($installedVersion, '0.3.5.6', '<')) { - \OC\Files\Cache\Shared_Updater::fixBrokenSharesOnAppUpdate(); } diff --git a/apps/files_sharing/tests/updater.php b/apps/files_sharing/tests/updater.php index 79ae4879b6..3427cfe388 100644 --- a/apps/files_sharing/tests/updater.php +++ b/apps/files_sharing/tests/updater.php @@ -20,6 +20,8 @@ * */ +require_once __DIR__ . '/../appinfo/update.php'; + /** * Class Test_Files_Sharing_Updater */ @@ -88,4 +90,57 @@ class Test_Files_Sharing_Updater extends \PHPUnit_Framework_TestCase { $result = $countItems->execute()->fetchOne(); $this->assertEquals(2, $result); } + + /** + * test update for the removal of the logical "Shared" folder. It should update + * the file_target for every share and create a physical "Shared" folder for each user + */ + function testRemoveSharedFolder() { + self::prepareDB(); + // run the update routine to remove the shared folder and replace it with a real folder + removeSharedFolder(false, 2); + + // verify results + $query = \OC_DB::prepare('SELECT * FROM `*PREFIX*share`'); + $result = $query->execute(array()); + + $newDBContent = $result->fetchAll(); + + foreach ($newDBContent as $row) { + if ((int)$row['share_type'] === \OCP\Share::SHARE_TYPE_USER) { + $this->assertSame('/Shared', substr($row['file_target'], 0, strlen('/Shared'))); + } else { + $this->assertSame('/ShouldNotChange', $row['file_target']); + } + } + + $this->cleanupSharedTable(); + + } + + private function cleanupSharedTable() { + $query = \OC_DB::prepare('DELETE FROM `*PREFIX*share`'); + $query->execute(); + } + + private function prepareDB() { + $this->cleanupSharedTable(); + // add items except one - because this is the test case for the broken share table + $addItems = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (`share_type`, `item_type`, ' . + '`share_with`, `uid_owner` , `file_target`) ' . + 'VALUES (?, ?, ?, ?, ?)'); + $items = array( + array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user1', 'admin' , '/foo'), + array(\OCP\Share::SHARE_TYPE_USER, 'folder', 'user2', 'admin', '/foo2'), + array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user3', 'admin', '/foo3'), + array(\OCP\Share::SHARE_TYPE_USER, 'folder', 'user4', 'admin', '/foo4'), + array(\OCP\Share::SHARE_TYPE_LINK, 'file', 'user1', 'admin', '/ShouldNotChange'), + array(\OCP\Share::SHARE_TYPE_CONTACT, 'contact', 'admin', 'user1', '/ShouldNotChange'), + + ); + foreach($items as $item) { + // the number is used as path_hash + $addItems->execute($item); + } + } } From 7ef8f6d352811e635bc6cf99b56d9482a54eb791 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 17 Apr 2014 15:54:45 +0200 Subject: [PATCH 33/34] always allow to rename the share mount point --- apps/files/js/filelist.js | 13 ++++++++++++- apps/files/lib/helper.php | 3 +++ apps/files_sharing/lib/cache.php | 4 ++++ lib/private/connector/sabre/objecttree.php | 8 +++++++- 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 343da21741..390bf4e057 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -178,6 +178,13 @@ window.FileList = { if (type === 'dir') { mime = mime || 'httpd/unix-directory'; } + + // user should always be able to rename a share mount point + var allowRename = 0; + if (fileData.isShareMountPoint) { + allowRename = OC.PERMISSION_UPDATE; + } + //containing tr var tr = $('').attr({ "data-id" : fileData.id, @@ -187,7 +194,7 @@ window.FileList = { "data-mime": mime, "data-mtime": mtime, "data-etag": fileData.etag, - "data-permissions": fileData.permissions || this.getDirectoryPermissions() + "data-permissions": fileData.permissions | allowRename || this.getDirectoryPermissions() }); if (type === 'dir') { @@ -283,6 +290,10 @@ window.FileList = { mime = fileData.mimetype, permissions = parseInt(fileData.permissions, 10) || 0; + if (fileData.isShareMountPoint) { + permissions = permissions | OC.PERMISSION_UPDATE; + } + if (type === 'dir') { mime = mime || 'httpd/unix-directory'; } diff --git a/apps/files/lib/helper.php b/apps/files/lib/helper.php index 88a5ffcfb6..0ae87d12fb 100644 --- a/apps/files/lib/helper.php +++ b/apps/files/lib/helper.php @@ -96,6 +96,9 @@ class Helper if (isset($i['displayname_owner'])) { $entry['shareOwner'] = $i['displayname_owner']; } + if (isset($i['is_share_mount_point'])) { + $entry['isShareMountPoint'] = $i['is_share_mount_point']; + } return $entry; } diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index 4a2f0ff08b..67a0410ef7 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -91,6 +91,9 @@ class Shared_Cache extends Cache { $data = $cache->get($this->files[$file]); $data['displayname_owner'] = \OC_User::getDisplayName($this->storage->getSharedFrom()); $data['path'] = $path; + if ($file === '') { + $data['is_share_mount_point'] = true; + } return $data; } } else { @@ -123,6 +126,7 @@ class Shared_Cache extends Cache { } if (isset($mountPoint)) { $data['path'] = 'files/' . $mountPoint; + $data['is_share_mount_point'] = true; } return $data; } diff --git a/lib/private/connector/sabre/objecttree.php b/lib/private/connector/sabre/objecttree.php index d2fa425b22..2956f60838 100644 --- a/lib/private/connector/sabre/objecttree.php +++ b/lib/private/connector/sabre/objecttree.php @@ -87,9 +87,15 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { list($sourceDir,) = \Sabre_DAV_URLUtil::splitPath($sourcePath); list($destinationDir,) = \Sabre_DAV_URLUtil::splitPath($destinationPath); + $isShareMountPoint = false; + list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath( '/' . \OCP\User::getUser() . '/files/' . $sourcePath); + if ($storage instanceof \OC\Files\Storage\Shared && !$internalPath) { + $isShareMountPoint = true; + } + // check update privileges $fs = $this->getFileView(); - if (!$fs->isUpdatable($sourcePath)) { + if (!$fs->isUpdatable($sourcePath) && !$isShareMountPoint) { throw new \Sabre_DAV_Exception_Forbidden(); } if ($sourceDir !== $destinationDir) { From b312d38d38c4e391765beb0aadb6bd2eafd9cb2c Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 23 Apr 2014 12:59:22 +0200 Subject: [PATCH 34/34] remove hard-coded shared folder --- lib/private/share/share.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/share/share.php b/lib/private/share/share.php index 4e3e261baf..4a76a010eb 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -223,7 +223,7 @@ class Share extends \OC\Share\Constants { } else { while ($row = $result->fetchRow()) { foreach ($fileTargets[$row['fileid']] as $uid => $shareData) { - $sharedPath = '/Shared' . $shareData['file_target']; + $sharedPath = $shareData['file_target']; $sharedPath .= substr($path, strlen($row['path']) -5); $sharePaths[$uid] = $sharedPath; }