diff options
Diffstat (limited to 'apps')
| -rw-r--r-- | apps/files_sharing/appinfo/app.php | 3 | ||||
| -rw-r--r-- | apps/files_sharing/lib/exceptions.php | 32 | ||||
| -rw-r--r-- | apps/files_sharing/lib/share/file.php | 9 | ||||
| -rw-r--r-- | apps/files_sharing/lib/sharedmount.php | 21 | ||||
| -rw-r--r-- | apps/files_sharing/lib/sharedstorage.php | 17 | ||||
| -rw-r--r-- | apps/files_sharing/tests/base.php | 12 | ||||
| -rw-r--r-- | apps/files_sharing/tests/share.php | 160 | ||||
| -rw-r--r-- | apps/files_sharing/tests/sharedmount.php | 37 |
8 files changed, 243 insertions, 48 deletions
diff --git a/apps/files_sharing/appinfo/app.php b/apps/files_sharing/appinfo/app.php index 543cae7b0c1..f2c454a9ae2 100644 --- a/apps/files_sharing/appinfo/app.php +++ b/apps/files_sharing/appinfo/app.php @@ -12,6 +12,9 @@ OC::$CLASSPATH['OCA\Files\Share\Api'] = 'files_sharing/lib/api.php'; OC::$CLASSPATH['OCA\Files\Share\Maintainer'] = 'files_sharing/lib/maintainer.php'; OC::$CLASSPATH['OCA\Files\Share\Proxy'] = 'files_sharing/lib/proxy.php'; +// Exceptions +OC::$CLASSPATH['OCA\Files_Sharing\Exceptions\BrokenPath'] = 'files_sharing/lib/exceptions.php'; + \OCP\App::registerAdmin('files_sharing', 'settings-admin'); \OCA\Files_Sharing\Helper::registerHooks(); diff --git a/apps/files_sharing/lib/exceptions.php b/apps/files_sharing/lib/exceptions.php new file mode 100644 index 00000000000..2a57a69a95f --- /dev/null +++ b/apps/files_sharing/lib/exceptions.php @@ -0,0 +1,32 @@ +<?php +/** + * ownCloud + * + * @author Bjoern Schiessle + * @copyright 2014 Bjoern Schiessle <schiessle@owncloud.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE + * License as published by the Free Software Foundation; either + * version 3 of the License, or any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU AFFERO GENERAL PUBLIC LICENSE for more details. + * + * You should have received a copy of the GNU Affero General Public + * License along with this library. If not, see <http://www.gnu.org/licenses/>. + * + */ + +namespace OCA\Files_Sharing\Exceptions; + +/** + * Expected path with a different root + * Possible Error Codes: + * 10 - Path not relative to data/ and point to the users file directory + + */ +class BrokenPath extends \Exception { +} diff --git a/apps/files_sharing/lib/share/file.php b/apps/files_sharing/lib/share/file.php index 2ae7fdc16ab..dacdb9308be 100644 --- a/apps/files_sharing/lib/share/file.php +++ b/apps/files_sharing/lib/share/file.php @@ -38,7 +38,7 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { // FIXME: attributes should not be set here, // keeping this pattern for now to avoid unexpected // regressions - $this->path = basename($path); + $this->path = \OC\Files\Filesystem::normalizePath(basename($path)); return true; } return false; @@ -57,7 +57,7 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { * create unique target * @param string $filePath * @param string $shareWith - * @param string $exclude + * @param array $exclude (optional) * @return string */ public function generateTarget($filePath, $shareWith, $exclude = null) { @@ -83,10 +83,7 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent { } } - $excludeList = \OCP\Share::getItemsSharedWithUser('file', $shareWith, self::FORMAT_TARGET_NAMES); - if (is_array($exclude)) { - $excludeList = array_merge($excludeList, $exclude); - } + $excludeList = (is_array($exclude)) ? $exclude : array(); return \OCA\Files_Sharing\Helper::generateUniqueTarget($target, $excludeList, $view); } diff --git a/apps/files_sharing/lib/sharedmount.php b/apps/files_sharing/lib/sharedmount.php index 564ac43ec74..4ad2d4e6b3e 100644 --- a/apps/files_sharing/lib/sharedmount.php +++ b/apps/files_sharing/lib/sharedmount.php @@ -58,7 +58,7 @@ class SharedMount extends Mount implements MoveableMount { * update fileTarget in the database if the mount point changed * @param string $newPath * @param array $share reference to the share which should be modified - * @return type + * @return bool */ private static function updateFileTarget($newPath, &$share) { // if the user renames a mount point from a group share we need to create a new db entry @@ -91,7 +91,7 @@ class SharedMount extends Mount implements MoveableMount { * @param string $path the absolute path * @return string e.g. turns '/admin/files/test.txt' into '/test.txt' */ - private function stripUserFilesPath($path) { + protected function stripUserFilesPath($path) { $trimmed = ltrim($path, '/'); $split = explode('/', $trimmed); @@ -99,8 +99,8 @@ class SharedMount extends Mount implements MoveableMount { 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; + \OCP\Util::ERROR); + throw new \OCA\Files_Sharing\Exceptions\BrokenPath('Path does not start with /user/files', 10); } // skip 'user' and 'files' @@ -121,7 +121,15 @@ class SharedMount extends Mount implements MoveableMount { $relTargetPath = $this->stripUserFilesPath($target); $share = $this->storage->getShare(); - $result = $this->updateFileTarget($relTargetPath, $share); + $result = true; + + if (!empty($share['grouped'])) { + foreach ($share['grouped'] as $s) { + $result = $this->updateFileTarget($relTargetPath, $s) && $result; + } + } else { + $result = $this->updateFileTarget($relTargetPath, $share) && $result; + } if ($result) { $this->setMountPoint($target); @@ -144,8 +152,9 @@ class SharedMount extends Mount implements MoveableMount { */ public function removeMount() { $mountManager = \OC\Files\Filesystem::getMountManager(); + /** @var \OC\Files\Storage\Shared */ $storage = $this->getStorage(); - $result = \OCP\Share::unshareFromSelf($storage->getItemType(), $storage->getMountPoint()); + $result = $storage->unshareStorage(); $mountManager->removeMount($this->mountPoint); return $result; diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index 07f6b9da10c..a2b485a2ca1 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -532,4 +532,21 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage { return null; } + /** + * unshare complete storage, also the grouped shares + * + * @return bool + */ + public function unshareStorage() { + $result = true; + if (!empty($this->share['grouped'])) { + foreach ($this->share['grouped'] as $share) { + $result = $result && \OCP\Share::unshareFromSelf($share['item_type'], $share['file_target']); + } + } + $result = $result && \OCP\Share::unshareFromSelf($this->getItemType(), $this->getMountPoint()); + + return $result; + } + } diff --git a/apps/files_sharing/tests/base.php b/apps/files_sharing/tests/base.php index 34ec4a36ede..738ba3493ba 100644 --- a/apps/files_sharing/tests/base.php +++ b/apps/files_sharing/tests/base.php @@ -35,6 +35,8 @@ abstract class Test_Files_Sharing_Base extends \PHPUnit_Framework_TestCase { const TEST_FILES_SHARING_API_USER2 = "test-share-user2"; const TEST_FILES_SHARING_API_USER3 = "test-share-user3"; + const TEST_FILES_SHARING_API_GROUP1 = "test-share-group1"; + public $stateFilesEncryption; public $filename; public $data; @@ -60,6 +62,10 @@ abstract class Test_Files_Sharing_Base extends \PHPUnit_Framework_TestCase { self::loginHelper(self::TEST_FILES_SHARING_API_USER2, true); self::loginHelper(self::TEST_FILES_SHARING_API_USER3, true); + // create group + \OC_Group::createGroup(self::TEST_FILES_SHARING_API_GROUP1); + \OC_Group::addToGroup(self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_GROUP1); + } function setUp() { @@ -86,6 +92,9 @@ abstract class Test_Files_Sharing_Base extends \PHPUnit_Framework_TestCase { } else { \OC_App::disable('files_encryption'); } + + $query = \OCP\DB::prepare('DELETE FROM `*PREFIX*share`'); + $query->execute(); } public static function tearDownAfterClass() { @@ -94,6 +103,9 @@ abstract class Test_Files_Sharing_Base extends \PHPUnit_Framework_TestCase { \OC_User::deleteUser(self::TEST_FILES_SHARING_API_USER1); \OC_User::deleteUser(self::TEST_FILES_SHARING_API_USER2); \OC_User::deleteUser(self::TEST_FILES_SHARING_API_USER3); + + // delete group + \OC_Group::deleteGroup(self::TEST_FILES_SHARING_API_GROUP1); } /** diff --git a/apps/files_sharing/tests/share.php b/apps/files_sharing/tests/share.php index 31246c5df44..fe80cfca781 100644 --- a/apps/files_sharing/tests/share.php +++ b/apps/files_sharing/tests/share.php @@ -57,6 +57,10 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { self::$tempStorage = null; + // clear database table + $query = \OCP\DB::prepare('DELETE FROM `*PREFIX*share`'); + $query->execute(); + parent::tearDown(); } @@ -68,10 +72,6 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { $fileinfo = $this->view->getFileInfo($this->filename); - $pathinfo = pathinfo($this->filename); - - $duplicate = '/' . $pathinfo['filename'] . ' (2).' . $pathinfo['extension']; - $result = \OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, \Test_Files_Sharing::TEST_FILES_SHARING_API_USER2, 31); @@ -84,47 +84,146 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { self::loginHelper(self::TEST_FILES_SHARING_API_USER2); $this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertTrue(\OC\Files\Filesystem::file_exists($duplicate)); self::loginHelper(self::TEST_FILES_SHARING_API_USER3); $this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertFalse(\OC\Files\Filesystem::file_exists($duplicate)); self::loginHelper(self::TEST_FILES_SHARING_API_USER2); \OC\Files\Filesystem::unlink($this->filename); self::loginHelper(self::TEST_FILES_SHARING_API_USER2); + // both group share and user share should be gone $this->assertFalse(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertTrue(\OC\Files\Filesystem::file_exists($duplicate)); // for user3 nothing should change self::loginHelper(self::TEST_FILES_SHARING_API_USER3); $this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertFalse(\OC\Files\Filesystem::file_exists($duplicate)); + } + + /** + * if a file was shared as group share and as individual share they should be grouped + */ + function testGroupingOfShares() { + + $fileinfo = $this->view->getFileInfo($this->filename); + + $result = \OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, + \Test_Files_Sharing::TEST_FILES_SHARING_API_GROUP1, \OCP\PERMISSION_READ); + + $this->assertTrue($result); + + $result = \OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing::TEST_FILES_SHARING_API_USER2, \OCP\PERMISSION_UPDATE); + + $this->assertTrue($result); self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - \OC\Files\Filesystem::unlink($duplicate); - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - $this->assertFalse(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertFalse(\OC\Files\Filesystem::file_exists($duplicate)); - // for user3 nothing should change - self::loginHelper(self::TEST_FILES_SHARING_API_USER3); - $this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename)); - $this->assertFalse(\OC\Files\Filesystem::file_exists($duplicate)); + $result = \OCP\Share::getItemSharedWith('file', null); + + $this->assertTrue(is_array($result)); + + // test should return exactly one shares created from testCreateShare() + $this->assertSame(1, count($result)); + + $share = reset($result); + $this->assertSame(\OCP\PERMISSION_READ | \OCP\PERMISSION_UPDATE, $share['permissions']); + + \OC\Files\Filesystem::rename($this->filename, $this->filename . '-renamed'); + + $result = \OCP\Share::getItemSharedWith('file', null); + + $this->assertTrue(is_array($result)); + + // test should return exactly one shares created from testCreateShare() + $this->assertSame(1, count($result)); + + $share = reset($result); + $this->assertSame(\OCP\PERMISSION_READ | \OCP\PERMISSION_UPDATE, $share['permissions']); + $this->assertSame($this->filename . '-renamed', $share['file_target']); - //cleanup self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - \OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, - 'testGroup'); - \OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, - self::TEST_FILES_SHARING_API_USER2); - \OC_Group::removeFromGroup(self::TEST_FILES_SHARING_API_USER2, 'testGroup'); - \OC_Group::removeFromGroup(self::TEST_FILES_SHARING_API_USER2, 'testGroup'); - \OC_Group::deleteGroup('testGroup'); + // unshare user share + $result = \OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, + \Test_Files_Sharing::TEST_FILES_SHARING_API_USER2); + $this->assertTrue($result); + + self::loginHelper(self::TEST_FILES_SHARING_API_USER2); + + $result = \OCP\Share::getItemSharedWith('file', null); + + $this->assertTrue(is_array($result)); + + // test should return the remaining group share + $this->assertSame(1, count($result)); + + $share = reset($result); + // only the group share permissions should be available now + $this->assertSame(\OCP\PERMISSION_READ, $share['permissions']); + $this->assertSame($this->filename . '-renamed', $share['file_target']); } + /** + * user1 share file to a group and to a user2 in the same group. Then user2 + * unshares the file from self. Afterwards user1 should no longer see the + * single user share to user2. If he re-shares the file to user2 the same target + * then the group share should be used to group the item + */ + function testShareAndUnshareFromSelf() { + $fileinfo = $this->view->getFileInfo($this->filename); + + // share the file to group1 (user2 is a member of this group) and explicitely to user2 + \OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, self::TEST_FILES_SHARING_API_GROUP1, \OCP\PERMISSION_ALL); + \OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, \OCP\PERMISSION_ALL); + + // user1 should have to shared files + $shares = \OCP\Share::getItemsShared('file'); + $this->assertSame(2, count($shares)); + + // user2 should have two files "welcome.txt" and the shared file, + // both the group share and the single share of the same file should be + // grouped to one file + \Test_Files_Sharing::loginHelper(self::TEST_FILES_SHARING_API_USER2); + $dirContent = \OC\Files\Filesystem::getDirectoryContent('/'); + $this->assertSame(2, count($dirContent)); + $this->verifyDirContent($dirContent, array('welcome.txt', ltrim($this->filename, '/'))); + + // now user2 deletes the share (= unshare from self) + \OC\Files\Filesystem::unlink($this->filename); + + // only welcome.txt should exists + $dirContent = \OC\Files\Filesystem::getDirectoryContent('/'); + $this->assertSame(1, count($dirContent)); + $this->verifyDirContent($dirContent, array('welcome.txt')); + + // login as user1... + \Test_Files_Sharing::loginHelper(self::TEST_FILES_SHARING_API_USER1); + + // ... now user1 should have only one shared file, the group share + $shares = \OCP\Share::getItemsShared('file'); + $this->assertSame(1, count($shares)); + + // user1 shares a gain the file directly to user2 + \OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, \OCP\PERMISSION_ALL); + + // user2 should see again welcome.txt and the shared file + \Test_Files_Sharing::loginHelper(self::TEST_FILES_SHARING_API_USER2); + $dirContent = \OC\Files\Filesystem::getDirectoryContent('/'); + $this->assertSame(2, count($dirContent)); + $this->verifyDirContent($dirContent, array('welcome.txt', ltrim($this->filename, '/'))); + + + } + + function verifyDirContent($content, $expected) { + foreach ($content as $c) { + if (!in_array($c['name'], $expected)) { + $this->assertTrue(false, "folder should only contain '" . implode(',', $expected) . "', found: " .$c['name']); + } + } + } + function testShareWithDifferentShareFolder() { $fileinfo = $this->view->getFileInfo($this->filename); @@ -146,12 +245,6 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { $this->assertTrue(\OC\Files\Filesystem::file_exists('/Shared/subfolder/' . $this->folder)); //cleanup - self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - \OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, - self::TEST_FILES_SHARING_API_USER2); - \OCP\Share::unshare('folder', $folderinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, - self::TEST_FILES_SHARING_API_USER2); - \OCP\Config::deleteSystemValue('share_folder'); } @@ -173,18 +266,14 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { $this->assertTrue(is_array($result)); // test should return exactly one shares created from testCreateShare() - $this->assertTrue(count($result) === 1); + $this->assertSame(1, count($result), 'more then one share found'); $share = reset($result); $this->assertSame($expectedPermissions, $share['permissions']); - - \OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, - \Test_Files_Sharing::TEST_FILES_SHARING_API_USER2); } function DataProviderTestFileSharePermissions() { $permission1 = \OCP\PERMISSION_ALL; - $permission2 = \OCP\PERMISSION_DELETE; $permission3 = \OCP\PERMISSION_READ; $permission4 = \OCP\PERMISSION_READ | \OCP\PERMISSION_UPDATE; $permission5 = \OCP\PERMISSION_READ | \OCP\PERMISSION_DELETE; @@ -192,7 +281,6 @@ class Test_Files_Sharing extends Test_Files_Sharing_Base { return array( array($permission1, \OCP\PERMISSION_ALL & ~\OCP\PERMISSION_DELETE), - array($permission2, 0), array($permission3, $permission3), array($permission4, $permission4), array($permission5, $permission3), diff --git a/apps/files_sharing/tests/sharedmount.php b/apps/files_sharing/tests/sharedmount.php index f8c65734184..ac910944f9f 100644 --- a/apps/files_sharing/tests/sharedmount.php +++ b/apps/files_sharing/tests/sharedmount.php @@ -194,4 +194,41 @@ class Test_Files_Sharing_Mount extends Test_Files_Sharing_Base { \OC_Group::removeFromGroup(self::TEST_FILES_SHARING_API_USER3, 'testGroup'); } + /** + * @dataProvider dataProviderTestStripUserFilesPath + * @param string $path + * @param string $expectedResult + * @param bool $exception if a exception is expected + */ + function testStripUserFilesPath($path, $expectedResult, $exception) { + $testClass = new DummyTestClassSharedMount(null, null); + try { + $result = $testClass->stripUserFilesPathDummy($path); + $this->assertSame($expectedResult, $result); + } catch (\Exception $e) { + if ($exception) { + $this->assertSame(10, $e->getCode()); + } else { + $this->assertTrue(false, "Exception catched, but expected: " . $expectedResult); + } + } + } + + function dataProviderTestStripUserFilesPath() { + return array( + array('/user/files/foo.txt', '/foo.txt', false), + array('/user/files/folder/foo.txt', '/folder/foo.txt', false), + array('/data/user/files/foo.txt', null, true), + array('/data/user/files/', null, true), + array('/files/foo.txt', null, true), + array('/foo.txt', null, true), + ); + } + +} + +class DummyTestClassSharedMount extends \OCA\Files_Sharing\SharedMount { + public function stripUserFilesPathDummy($path) { + return $this->stripUserFilesPath($path); + } } |