Fix shared storage permission checks

diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php
index ccfbebddb294726038ffc960b2dc68fad0a68f08..51b3f36d534f297d4f1b02dac2d6ff904e7e42c3 100644 (file)
--- 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 2959b9aacfb1822fa3d520b8d6b4a746a45f34a5..2b057bd20af1bb8b65d9fd5c3052b0255d1028f6 100644 (file)
--- 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);