wbrawner/server
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix shared storage permission checks

Browse source
This commit is contained in:
Vincent Petry 2015-01-30 14:19:23 +01:00
parent fe8002a7db
commit c2315aa015
2 changed files with 166 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
apps/files_sharing/lib/sharedstorage.php
View file

@ -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,

148
apps/files_sharing/tests/sharedstorage.php
View file

@ -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);