[Pkg-owncloud-commits] [owncloud] 99/118: Fix shared storage permission checks
David Prévot
taffit at moszumanska.debian.org
Fri Mar 27 22:13:18 UTC 2015
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch stable8
in repository owncloud.
commit 5ee843cd5930f5e90e3d983843743c29714dcb97
Author: Vincent Petry <pvince81 at owncloud.com>
Date: Fri Jan 30 14:19:23 2015 +0100
Fix shared storage permission checks
---
apps/files_sharing/lib/sharedstorage.php | 22 ++++-
apps/files_sharing/tests/sharedstorage.php | 148 +++++++++++++++++++++++++++++
2 files changed, 166 insertions(+), 4 deletions(-)
diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php
index d992f8f..2e8eac3 100644
--- a/apps/files_sharing/lib/sharedstorage.php
+++ b/apps/files_sharing/lib/sharedstorage.php
@@ -294,8 +294,10 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage {
$relPath1 = $this->getMountPoint() . '/' . $path1;
$relPath2 = $this->getMountPoint() . '/' . $path2;
+ $targetExists = $this->file_exists($path2);
+
// check for update permissions on the share
- if ($this->isUpdatable('')) {
+ if (($targetExists && $this->isUpdatable('')) || (!$targetExists && $this->isCreatable(''))) {
$pathinfo = pathinfo($relPath1);
// for part files we need to ask for the owner and path from the parent directory because
@@ -343,13 +345,25 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage {
case 'xb':
case 'a':
case 'ab':
- $exists = $this->file_exists($path);
- if ($exists && !$this->isUpdatable($path)) {
+ $creatable = $this->isCreatable($path);
+ $updatable = $this->isUpdatable($path);
+ // if neither permissions given, no need to continue
+ if (!$creatable && !$updatable) {
return false;
}
- if (!$exists && !$this->isCreatable(dirname($path))) {
+
+ $exists = $this->file_exists($path);
+ // if a file exists, updatable permissions are required
+ if ($exists && !$updatable) {
return false;
}
+
+ // part file is allowed if !$creatable but the final file is $updatable
+ if (pathinfo($path, PATHINFO_EXTENSION) !== 'part') {
+ if (!$exists && !$creatable) {
+ return false;
+ }
+ }
}
$info = array(
'target' => $this->getMountPoint() . $path,
diff --git a/apps/files_sharing/tests/sharedstorage.php b/apps/files_sharing/tests/sharedstorage.php
index 2959b9a..2b057bd 100644
--- a/apps/files_sharing/tests/sharedstorage.php
+++ b/apps/files_sharing/tests/sharedstorage.php
@@ -199,6 +199,154 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase {
$this->assertTrue($result);
}
+ function testFopenWithReadOnlyPermission() {
+ $this->view->file_put_contents($this->folder . '/existing.txt', 'foo');
+ $fileinfoFolder = $this->view->getFileInfo($this->folder);
+ $result = \OCP\Share::shareItem('folder', $fileinfoFolder['fileid'], \OCP\Share::SHARE_TYPE_USER,
+ self::TEST_FILES_SHARING_API_USER2, \OCP\Constants::PERMISSION_READ);
+ $this->assertTrue($result);
+
+ self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
+ $user2View = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files');
+
+ // part file should be forbidden
+ $handle = $user2View->fopen($this->folder . '/test.txt.part', 'w');
+ $this->assertFalse($handle);
+
+ // regular file forbidden
+ $handle = $user2View->fopen($this->folder . '/test.txt', 'w');
+ $this->assertFalse($handle);
+
+ // rename forbidden
+ $this->assertFalse($user2View->rename($this->folder . '/existing.txt', $this->folder . '/existing2.txt'));
+
+ // delete forbidden
+ $this->assertFalse($user2View->unlink($this->folder . '/existing.txt'));
+
+ //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);
+ }
+
+ function testFopenWithCreateOnlyPermission() {
+ $this->view->file_put_contents($this->folder . '/existing.txt', 'foo');
+ $fileinfoFolder = $this->view->getFileInfo($this->folder);
+ $result = \OCP\Share::shareItem('folder', $fileinfoFolder['fileid'], \OCP\Share::SHARE_TYPE_USER,
+ self::TEST_FILES_SHARING_API_USER2, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE);
+ $this->assertTrue($result);
+
+ self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
+ $user2View = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files');
+
+ // create part file allowed
+ $handle = $user2View->fopen($this->folder . '/test.txt.part', 'w');
+ $this->assertNotFalse($handle);
+ fclose($handle);
+
+ // create regular file allowed
+ $handle = $user2View->fopen($this->folder . '/test-create.txt', 'w');
+ $this->assertNotFalse($handle);
+ fclose($handle);
+
+ // rename file allowed as long as target did not exist
+ $this->assertTrue($user2View->rename($this->folder . '/test-create.txt', $this->folder . '/newtarget.txt'));
+ $this->assertTrue($user2View->file_exists($this->folder . '/newtarget.txt'));
+
+ // rename file not allowed if target exists
+ $this->assertFalse($user2View->rename($this->folder . '/newtarget.txt', $this->folder . '/existing.txt'));
+
+ // overwriting file not allowed
+ $handle = $user2View->fopen($this->folder . '/existing.txt', 'w');
+ $this->assertFalse($handle);
+
+ // overwrite forbidden (no update permission)
+ $this->assertFalse($user2View->rename($this->folder . '/test.txt.part', $this->folder . '/existing.txt'));
+
+ // delete forbidden
+ $this->assertFalse($user2View->unlink($this->folder . '/existing.txt'));
+
+ //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);
+ }
+
+ function testFopenWithUpdateOnlyPermission() {
+ $this->view->file_put_contents($this->folder . '/existing.txt', 'foo');
+ $fileinfoFolder = $this->view->getFileInfo($this->folder);
+
+ $result = \OCP\Share::shareItem('folder', $fileinfoFolder['fileid'], \OCP\Share::SHARE_TYPE_USER,
+ self::TEST_FILES_SHARING_API_USER2, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE);
+ $this->assertTrue($result);
+
+ self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
+ $user2View = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files');
+
+ // create part file allowed
+ $handle = $user2View->fopen($this->folder . '/test.txt.part', 'w');
+ $this->assertNotFalse($handle);
+ fclose($handle);
+
+ // create regular file not allowed
+ $handle = $user2View->fopen($this->folder . '/test-create.txt', 'w');
+ $this->assertFalse($handle);
+
+ // rename part file not allowed to non-existing file
+ $this->assertFalse($user2View->rename($this->folder . '/test.txt.part', $this->folder . '/nonexist.txt'));
+
+ // rename part file allowed to target existing file
+ $this->assertTrue($user2View->rename($this->folder . '/test.txt.part', $this->folder . '/existing.txt'));
+ $this->assertTrue($user2View->file_exists($this->folder . '/existing.txt'));
+
+ // overwriting file directly is allowed
+ $handle = $user2View->fopen($this->folder . '/existing.txt', 'w');
+ $this->assertNotFalse($handle);
+ fclose($handle);
+
+ // delete forbidden
+ $this->assertFalse($user2View->unlink($this->folder . '/existing.txt'));
+
+ //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);
+ }
+
+ function testFopenWithDeleteOnlyPermission() {
+ $this->view->file_put_contents($this->folder . '/existing.txt', 'foo');
+ $fileinfoFolder = $this->view->getFileInfo($this->folder);
+ $result = \OCP\Share::shareItem('folder', $fileinfoFolder['fileid'], \OCP\Share::SHARE_TYPE_USER,
+ self::TEST_FILES_SHARING_API_USER2, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_DELETE);
+ $this->assertTrue($result);
+
+ self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
+ $user2View = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files');
+
+ // part file should be forbidden
+ $handle = $user2View->fopen($this->folder . '/test.txt.part', 'w');
+ $this->assertFalse($handle);
+
+ // regular file forbidden
+ $handle = $user2View->fopen($this->folder . '/test.txt', 'w');
+ $this->assertFalse($handle);
+
+ // rename forbidden
+ $this->assertFalse($user2View->rename($this->folder . '/existing.txt', $this->folder . '/existing2.txt'));
+
+ // delete allowed
+ $this->assertTrue($user2View->unlink($this->folder . '/existing.txt'));
+
+ //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);
+ }
+
function testMountSharesOtherUser() {
$folderInfo = $this->view->getFileInfo($this->folder);
$fileInfo = $this->view->getFileInfo($this->filename);
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list