From 84d36df70b7bfb5e7cf79c7a089675d79943084d Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 24 Jun 2014 13:39:37 +0200 Subject: [PATCH 1/5] make sure that we always return the correct permissions --- apps/files_sharing/lib/cache.php | 6 +++--- lib/private/files/view.php | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/files_sharing/lib/cache.php b/apps/files_sharing/lib/cache.php index 7a6b70d82b..6bc9e13194 100644 --- a/apps/files_sharing/lib/cache.php +++ b/apps/files_sharing/lib/cache.php @@ -95,9 +95,9 @@ class Shared_Cache extends Cache { } $data['uid_owner'] = $this->storage->getOwner($file); if (isset($data['permissions'])) { - $data['permissions'] &= $this->storage->getPermissions(''); + $data['permissions'] &= $this->storage->getPermissions($file); } else { - $data['permissions'] = $this->storage->getPermissions(''); + $data['permissions'] = $this->storage->getPermissions($file); } return $data; } @@ -163,7 +163,7 @@ class Shared_Cache extends Cache { $sourceFolderContent[$key]['path'] = $dir . $c['name']; $sourceFolderContent[$key]['uid_owner'] = $parent['uid_owner']; $sourceFolderContent[$key]['displayname_owner'] = $parent['uid_owner']; - $sourceFolderContent[$key]['permissions'] = $sourceFolderContent[$key]['permissions'] & $this->storage->getPermissions(''); + $sourceFolderContent[$key]['permissions'] = $sourceFolderContent[$key]['permissions'] & $this->storage->getPermissions($dir . $c['name']); } return $sourceFolderContent; diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 91d7ea260d..48e7cb85be 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -884,8 +884,8 @@ class View { if ($extOnly && $subStorage instanceof \OC\Files\Storage\Shared) { continue; } - $subCache = $subStorage->getCache(''); - $rootEntry = $subCache->get(''); + $subCache = $subStorage->getCache($internalPath); + $rootEntry = $subCache->get($internalPath); $data['size'] += isset($rootEntry['size']) ? $rootEntry['size'] : 0; } } From 70ca292fd9c74a6f78983ae252729ee6fa69b1f4 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 24 Jun 2014 13:55:04 +0200 Subject: [PATCH 2/5] only the mount point has delete and update permissions to enable rename operation --- lib/private/files/view.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 48e7cb85be..c9c1fc3c1b 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -896,7 +896,7 @@ class View { return false; } - if ($mount instanceof MoveableMount) { + if ($mount instanceof MoveableMount && $internalPath === '') { $data['permissions'] |= \OCP\PERMISSION_DELETE | \OCP\PERMISSION_UPDATE; } From 3fc7357adea018878f5e1d41ea3c77d34ed73a98 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 24 Jun 2014 14:00:15 +0200 Subject: [PATCH 3/5] add/update unit tests --- apps/files_sharing/tests/permissions.php | 5 ++-- apps/files_sharing/tests/sharedstorage.php | 31 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/apps/files_sharing/tests/permissions.php b/apps/files_sharing/tests/permissions.php index 2cbc412d26..299e471a3f 100644 --- a/apps/files_sharing/tests/permissions.php +++ b/apps/files_sharing/tests/permissions.php @@ -145,10 +145,9 @@ class Test_Files_Sharing_Permissions extends Test_Files_Sharing_Base { $this->assertEquals(27, $contents[1]['permissions']); $contents = $this->secondView->getDirectoryContent('files/shareddirrestricted'); $this->assertEquals('subdir', $contents[0]['name']); - $this->assertEquals(7 | \OCP\PERMISSION_DELETE, $contents[0]['permissions']); + $this->assertEquals(7, $contents[0]['permissions']); $this->assertEquals('textfile1.txt', $contents[1]['name']); // 3 is correct because create is reserved to folders only - // delete permissions are added since mount points can always be deleted - $this->assertEquals(3 | \OCP\PERMISSION_DELETE, $contents[1]['permissions']); + $this->assertEquals(3, $contents[1]['permissions']); } } diff --git a/apps/files_sharing/tests/sharedstorage.php b/apps/files_sharing/tests/sharedstorage.php index b80ab6b4f1..27f3b5150d 100644 --- a/apps/files_sharing/tests/sharedstorage.php +++ b/apps/files_sharing/tests/sharedstorage.php @@ -166,4 +166,35 @@ class Test_Files_Sharing_Storage extends Test_Files_Sharing_Base { $this->assertTrue($result); } + function testGetPermissions() { + $fileinfoFolder = $this->view->getFileInfo($this->folder); + + $result = \OCP\Share::shareItem('folder', $fileinfoFolder['fileid'], \OCP\Share::SHARE_TYPE_USER, + self::TEST_FILES_SHARING_API_USER2, 1); + $this->assertTrue($result); + + self::loginHelper(self::TEST_FILES_SHARING_API_USER2); + + $this->assertTrue(\OC\Files\Filesystem::is_dir($this->folder)); + + // for the share root we expect: + // the shared permissions (1) + // the delete permission (8), to enable unshare + // the update permission (2), to allow renaming of the mount point + $rootInfo = \OC\Files\Filesystem::getFileInfo($this->folder); + $this->assertSame(11, $rootInfo->getPermissions()); + + // for the file within the shared folder we expect: + // the shared permissions (1) + $subfileInfo = \OC\Files\Filesystem::getFileInfo($this->folder . $this->filename); + $this->assertSame(1, $subfileInfo->getPermissions()); + + + //cleanup + self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + $result = \OCP\Share::unshare('folder', $fileinfoFolder['fileid'], \OCP\Share::SHARE_TYPE_USER, + self::TEST_FILES_SHARING_API_USER2); + $this->assertTrue($result); + } + } From 92a82f02b3f7e5422080fcaea33d89560ec260fb Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 24 Jun 2014 18:53:51 +0200 Subject: [PATCH 4/5] always take root of the substorage --- lib/private/files/view.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index c9c1fc3c1b..51eefb9dac 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -884,7 +884,7 @@ class View { if ($extOnly && $subStorage instanceof \OC\Files\Storage\Shared) { continue; } - $subCache = $subStorage->getCache($internalPath); + $subCache = $subStorage->getCache(''); $rootEntry = $subCache->get($internalPath); $data['size'] += isset($rootEntry['size']) ? $rootEntry['size'] : 0; } From 113749bd96811312ef390d0e760e6206bba90e46 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 25 Jun 2014 13:00:11 +0200 Subject: [PATCH 5/5] really always take root of the substorage --- lib/private/files/view.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 51eefb9dac..ff3cb9ee68 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -885,7 +885,7 @@ class View { continue; } $subCache = $subStorage->getCache(''); - $rootEntry = $subCache->get($internalPath); + $rootEntry = $subCache->get(''); $data['size'] += isset($rootEntry['size']) ? $rootEntry['size'] : 0; } }