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

Merge pull request #13802 from owncloud/share-partfilepermissions

Browse source 
Fix share permission checks
This commit is contained in:
Morris Jobke 2015-03-26 22:01:05 +01:00
commit e8109f0bc3
5 changed files with 224 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -299,26 +299,34 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage {
// we need the paths relative to data/user/files
$relPath1 = $this->getMountPoint() . '/' . $path1;
$relPath2 = $this->getMountPoint() . '/' . $path2;
$pathinfo = pathinfo($relPath1);
// check for update permissions on the share
if ($this->isUpdatable('')) {
$pathinfo = pathinfo($relPath1);
// for part files we need to ask for the owner and path from the parent directory because
// the file cache doesn't return any results for part files
if (isset($pathinfo['extension']) && $pathinfo['extension'] === 'part') {
list($user1, $path1) = \OCA\Files_Sharing\Helper::getUidAndFilename($pathinfo['dirname']);
$path1 = $path1 . '/' . $pathinfo['basename'];
} else {
list($user1, $path1) = \OCA\Files_Sharing\Helper::getUidAndFilename($relPath1);
$isPartFile = (isset($pathinfo['extension']) && $pathinfo['extension'] === 'part');
$targetExists = $this->file_exists($path2);
$sameFolder = (dirname($relPath1) === dirname($relPath2));
if ($targetExists || ($sameFolder && !$isPartFile)) {
// note that renaming a share mount point is always allowed
if (!$this->isUpdatable('')) {
return false;
}
} else {
if (!$this->isCreatable('')) {
return false;
}
$targetFilename = basename($relPath2);
list($user2, $path2) = \OCA\Files_Sharing\Helper::getUidAndFilename(dirname($relPath2));
$rootView = new \OC\Files\View('');
return $rootView->rename('/' . $user1 . '/files/' . $path1, '/' . $user2 . '/files/' . $path2 . '/' . $targetFilename);
}
return false;
// for part files we need to ask for the owner and path from the parent directory because
// the file cache doesn't return any results for part files
if ($isPartFile) {
list($user1, $path1) = \OCA\Files_Sharing\Helper::getUidAndFilename($pathinfo['dirname']);
$path1 = $path1 . '/' . $pathinfo['basename'];
} else {
list($user1, $path1) = \OCA\Files_Sharing\Helper::getUidAndFilename($relPath1);
}
$targetFilename = basename($relPath2);
list($user2, $path2) = \OCA\Files_Sharing\Helper::getUidAndFilename(dirname($relPath2));
$rootView = new \OC\Files\View('');
return $rootView->rename('/' . $user1 . '/files/' . $path1, '/' . $user2 . '/files/' . $path2 . '/' . $targetFilename);
}
public function copy($path1, $path2) {
@ -349,13 +357,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,

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

@ -202,6 +202,158 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase {
$this->assertTrue($result);
}
public 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);
}
public 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 never allowed
$this->assertFalse($user2View->rename($this->folder . '/test-create.txt', $this->folder . '/newtarget.txt'));
$this->assertFalse($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);
}
public 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'));
// rename regular file allowed
$this->assertTrue($user2View->rename($this->folder . '/existing.txt', $this->folder . '/existing-renamed.txt'));
$this->assertTrue($user2View->file_exists($this->folder . '/existing-renamed.txt'));
// overwriting file directly is allowed
$handle = $user2View->fopen($this->folder . '/existing-renamed.txt', 'w');
$this->assertNotFalse($handle);
fclose($handle);
// delete forbidden
$this->assertFalse($user2View->unlink($this->folder . '/existing-renamed.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);
}
public 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);

16
apps/files_trashbin/lib/storage.php
View file

@ -62,6 +62,22 @@ class Storage extends Wrapper {
self::$disableTrash = false;
}
/**
* Rename path1 to path2 by calling the wrapped storage.
*
* @param string $path1 first path
* @param string $path2 second path
*/
public function rename($path1, $path2) {
$result = $this->storage->rename($path1, $path2);
if ($result === false) {
// when rename failed, the post_rename hook isn't triggered,
// but we still want to reenable the trash logic
self::$disableTrash = false;
}
return $result;
}
/**
* Deletes the given file by moving it into the trashbin.
*

21
lib/private/connector/sabre/objecttree.php
View file

@ -175,8 +175,9 @@ class ObjectTree extends \Sabre\DAV\Tree {
throw new \Sabre\DAV\Exception\ServiceUnavailable('filesystem not setup');
}
$targetNodeExists = $this->nodeExists($destinationPath);
$sourceNode = $this->getNodeForPath($sourcePath);
if ($sourceNode instanceof \Sabre\DAV\ICollection and $this->nodeExists($destinationPath)) {
if ($sourceNode instanceof \Sabre\DAV\ICollection && $targetNodeExists) {
throw new \Sabre\DAV\Exception\Forbidden('Could not copy directory ' . $sourceNode . ', target exists');
}
list($sourceDir,) = \Sabre\HTTP\URLUtil::splitPath($sourcePath);
@ -190,14 +191,22 @@ class ObjectTree extends \Sabre\DAV\Tree {
}
try {
// check update privileges
if (!$this->fileView->isUpdatable($sourcePath) && !$isMovableMount) {
throw new \Sabre\DAV\Exception\Forbidden();
}
if ($sourceDir !== $destinationDir) {
$sameFolder = ($sourceDir === $destinationDir);
// if we're overwriting or same folder
if ($targetNodeExists || $sameFolder) {
// note that renaming a share mount point is always allowed
if (!$this->fileView->isUpdatable($destinationDir) && !$isMovableMount) {
throw new \Sabre\DAV\Exception\Forbidden();
}
} else {
if (!$this->fileView->isCreatable($destinationDir)) {
throw new \Sabre\DAV\Exception\Forbidden();
}
}
if (!$sameFolder) {
// moving to a different folder, source will be gone, like a deletion
// note that moving a share mount point is always allowed
if (!$this->fileView->isDeletable($sourcePath) && !$isMovableMount) {
throw new \Sabre\DAV\Exception\Forbidden();
}

4
tests/lib/connector/sabre/objecttree.php
View file

@ -69,7 +69,7 @@ class ObjectTree extends \Test\TestCase {
function moveFailedInvalidCharsProvider() {
return array(
array('a/b', 'a/*', array('a' => false, 'a/b' => true, 'a/c*' => false), array()),
array('a/b', 'a/*', array('a' => true, 'a/b' => true, 'a/c*' => false), array()),
);
}
@ -80,12 +80,12 @@ class ObjectTree extends \Test\TestCase {
array('a/b', 'b/b', array('a' => false, 'a/b' => true, 'b' => false, 'b/b' => false), array()),
array('a/b', 'b/b', array('a' => true, 'a/b' => true, 'b' => false, 'b/b' => false), array()),
array('a/b', 'b/b', array('a' => true, 'a/b' => true, 'b' => true, 'b/b' => false), array('a/b' => false)),
array('a/b', 'a/c', array('a' => false, 'a/b' => true, 'a/c' => false), array()),
);
}
function moveSuccessProvider() {
return array(
array('a/b', 'a/c', array('a' => false, 'a/b' => true, 'a/c' => false), array()),
array('a/b', 'b/b', array('a' => true, 'a/b' => true, 'b' => true, 'b/b' => false), array('a/b' => true)),
// older files with special chars can still be renamed to valid names
array('a/b*', 'b/b', array('a' => true, 'a/b*' => true, 'b' => true, 'b/b' => false), array('a/b*' => true)),