From 02ee4f958c4a5c99274964c609aeae9845ed37d3 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 2 Dec 2016 17:59:02 +0100 Subject: [PATCH 1/2] apply permissions mask for shared storage Signed-off-by: Robin Appelman --- apps/files_sharing/lib/SharedStorage.php | 18 ++++++++++++++---- apps/files_sharing/tests/SharedStorageTest.php | 4 ++-- .../Files/Storage/Wrapper/PermissionsMask.php | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/apps/files_sharing/lib/SharedStorage.php b/apps/files_sharing/lib/SharedStorage.php index 5b4aa06180..ad250a790f 100644 --- a/apps/files_sharing/lib/SharedStorage.php +++ b/apps/files_sharing/lib/SharedStorage.php @@ -33,6 +33,8 @@ namespace OCA\Files_Sharing; use OC\Files\Filesystem; use OC\Files\Cache\FailedCache; +use OC\Files\Storage\Wrapper\PermissionsMask; +use OCA\Files_Sharing\ISharedStorage; use OC\Files\Storage\FailedStorage; use OCP\Constants; use OCP\Files\Cache\ICacheEntry; @@ -71,6 +73,9 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto */ private $logger; + /** @var IStorage */ + private $nonMaskedStorage; + private $options; public function __construct($arguments) { @@ -94,7 +99,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto private function getSourceRootInfo() { if (is_null($this->sourceRootInfo)) { if (is_null($this->superShare->getNodeCacheEntry())) { - $this->sourceRootInfo = $this->getWrapperStorage()->getCache()->get($this->rootPath); + $this->sourceRootInfo = $this->nonMaskedStorage->getCache()->get($this->rootPath); } else { $this->sourceRootInfo = $this->superShare->getNodeCacheEntry(); } @@ -110,7 +115,11 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto try { Filesystem::initMountPoints($this->superShare->getShareOwner()); $sourcePath = $this->ownerView->getPath($this->superShare->getNodeId()); - list($this->storage, $this->rootPath) = $this->ownerView->resolvePath($sourcePath); + list($this->nonMaskedStorage, $this->rootPath) = $this->ownerView->resolvePath($sourcePath); + $this->storage = new PermissionsMask([ + 'storage' => $this->nonMaskedStorage, + 'mask' => $this->superShare->getPermissions() + ]); } catch (NotFoundException $e) { $this->storage = new FailedStorage(['exception' => $e]); $this->rootPath = ''; @@ -252,7 +261,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto 'mode' => $mode, ); \OCP\Util::emitHook('\OC\Files\Storage\Shared', 'fopen', $info); - return parent::fopen($path, $mode); + return $this->nonMaskedStorage->fopen($this->getSourcePath($path), $mode); } return false; } @@ -265,6 +274,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto * @return bool */ public function rename($path1, $path2) { + $this->init(); $isPartFile = pathinfo($path1, PATHINFO_EXTENSION) === 'part'; $targetExists = $this->file_exists($path2); $sameFodler = dirname($path1) === dirname($path2); @@ -279,7 +289,7 @@ class SharedStorage extends \OC\Files\Storage\Wrapper\Jail implements ISharedSto } } - return parent::rename($path1, $path2); + return $this->nonMaskedStorage->rename($this->getSourcePath($path1), $this->getSourcePath($path2)); } /** diff --git a/apps/files_sharing/tests/SharedStorageTest.php b/apps/files_sharing/tests/SharedStorageTest.php index 2486e23af1..f1b0cbb8fb 100644 --- a/apps/files_sharing/tests/SharedStorageTest.php +++ b/apps/files_sharing/tests/SharedStorageTest.php @@ -202,13 +202,13 @@ class SharedStorageTest extends TestCase { $this->assertTrue(\OC\Files\Filesystem::is_dir($this->folder)); // for the share root we expect: - // the shared permissions (1) + // the read permissions (1) // the delete permission (8), to enable unshare $rootInfo = \OC\Files\Filesystem::getFileInfo($this->folder); $this->assertSame(9, $rootInfo->getPermissions()); // for the file within the shared folder we expect: - // the shared permissions (1) + // the read permissions (1) $subfileInfo = \OC\Files\Filesystem::getFileInfo($this->folder . $this->filename); $this->assertSame(1, $subfileInfo->getPermissions()); diff --git a/lib/private/Files/Storage/Wrapper/PermissionsMask.php b/lib/private/Files/Storage/Wrapper/PermissionsMask.php index 7bcb1087fe..239824b67e 100644 --- a/lib/private/Files/Storage/Wrapper/PermissionsMask.php +++ b/lib/private/Files/Storage/Wrapper/PermissionsMask.php @@ -112,7 +112,7 @@ class PermissionsMask extends Wrapper { public function file_put_contents($path, $data) { $permissions = $this->file_exists($path) ? Constants::PERMISSION_UPDATE : Constants::PERMISSION_CREATE; - return $this->checkMask($permissions) and parent::file_put_contents($path, $data); + return $this->checkMask($permissions) ? parent::file_put_contents($path, $data) : false; } public function fopen($path, $mode) { From 1a379b0fdcb4ad468eab11e3c72d7f13ceeb42ea Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 2 Dec 2016 18:04:21 +0100 Subject: [PATCH 2/2] update test Signed-off-by: Robin Appelman --- tests/lib/Files/Storage/Wrapper/PermissionsMaskTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/Files/Storage/Wrapper/PermissionsMaskTest.php b/tests/lib/Files/Storage/Wrapper/PermissionsMaskTest.php index c342155364..9859915e2c 100644 --- a/tests/lib/Files/Storage/Wrapper/PermissionsMaskTest.php +++ b/tests/lib/Files/Storage/Wrapper/PermissionsMaskTest.php @@ -77,7 +77,7 @@ class PermissionsMaskTest extends \Test\Files\Storage\Storage { public function testPutContentsNewFileNoUpdate() { $storage = $this->getMaskedStorage(Constants::PERMISSION_ALL - Constants::PERMISSION_UPDATE); - $this->assertTrue($storage->file_put_contents('foo', 'bar')); + $this->assertEquals(3, $storage->file_put_contents('foo', 'bar')); $this->assertEquals('bar', $storage->file_get_contents('foo')); }