From ee29ab1f65a63b6d386bc1ddbd72f0e505d7f9a7 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 10 May 2023 15:20:56 +0200 Subject: [PATCH] fix share roots always being marked as writable Signed-off-by: Robin Appelman --- .../tests/unit/Connector/Sabre/NodeTest.php | 76 +++++++++++++------ lib/public/Files/DavUtil.php | 18 ++++- 2 files changed, 70 insertions(+), 24 deletions(-) diff --git a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php index 3ac5b8f841a08..1d09919739e0a 100644 --- a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php @@ -25,14 +25,20 @@ * along with this program. If not, see * */ + namespace OCA\DAV\Tests\unit\Connector\Sabre; use OC\Files\FileInfo; +use OC\Files\Mount\MountPoint; use OC\Files\View; use OC\Share20\ShareAttributes; +use OCA\Files_Sharing\SharedMount; use OCA\Files_Sharing\SharedStorage; +use OCP\Constants; +use OCP\Files\Cache\ICacheEntry; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage; +use OCP\ICache; use OCP\Share\IAttributes; use OCP\Share\IManager; use OCP\Share\IShare; @@ -46,40 +52,66 @@ class NodeTest extends \Test\TestCase { public function davPermissionsProvider() { return [ - [\OCP\Constants::PERMISSION_ALL, 'file', false, false, 'RGDNVW'], - [\OCP\Constants::PERMISSION_ALL, 'dir', false, false, 'RGDNVCK'], - [\OCP\Constants::PERMISSION_ALL, 'file', true, false, 'SRGDNVW'], - [\OCP\Constants::PERMISSION_ALL, 'file', true, true, 'SRMGDNVW'], - [\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_SHARE, 'file', true, false, 'SGDNVW'], - [\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_UPDATE, 'file', false, false, 'RGD'], - [\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_DELETE, 'file', false, false, 'RGNVW'], - [\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_CREATE, 'file', false, false, 'RGDNVW'], - [\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_READ, 'file', false, false, 'RDNVW'], - [\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_CREATE, 'dir', false, false, 'RGDNV'], - [\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_READ, 'dir', false, false, 'RDNVCK'], + [Constants::PERMISSION_ALL, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGDNVW'], + [Constants::PERMISSION_ALL, 'dir', false, Constants::PERMISSION_ALL, false, 'test', 'RGDNVCK'], + [Constants::PERMISSION_ALL, 'file', true, Constants::PERMISSION_ALL, false, 'test', 'SRGDNVW'], + [Constants::PERMISSION_ALL, 'file', true, Constants::PERMISSION_ALL, true, 'test', 'SRMGDNVW'], + [Constants::PERMISSION_ALL, 'file', true, Constants::PERMISSION_ALL, true, '' , 'SRMGDNVW'], + [Constants::PERMISSION_ALL, 'file', true, Constants::PERMISSION_ALL - Constants::PERMISSION_UPDATE, true, '' , 'SRMGDNV'], + [Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE, 'file', true, Constants::PERMISSION_ALL, false, 'test', 'SGDNVW'], + [Constants::PERMISSION_ALL - Constants::PERMISSION_UPDATE, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGD'], + [Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGNVW'], + [Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGDNVW'], + [Constants::PERMISSION_ALL - Constants::PERMISSION_READ, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RDNVW'], + [Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE, 'dir', false, Constants::PERMISSION_ALL, false, 'test', 'RGDNV'], + [Constants::PERMISSION_ALL - Constants::PERMISSION_READ, 'dir', false, Constants::PERMISSION_ALL, false, 'test', 'RDNVCK'], ]; } /** * @dataProvider davPermissionsProvider */ - public function testDavPermissions($permissions, $type, $shared, $mounted, $expected) { + public function testDavPermissions($permissions, $type, $shared, $shareRootPermissions, $mounted, $internalPath, $expected) { $info = $this->getMockBuilder(FileInfo::class) ->disableOriginalConstructor() - ->setMethods(['getPermissions', 'isShared', 'isMounted', 'getType']) + ->onlyMethods(['getPermissions', 'isShared', 'isMounted', 'getType', 'getInternalPath', 'getStorage', 'getMountPoint']) ->getMock(); - $info->expects($this->any()) - ->method('getPermissions') + $info->method('getPermissions') ->willReturn($permissions); - $info->expects($this->any()) - ->method('isShared') + $info->method('isShared') ->willReturn($shared); - $info->expects($this->any()) - ->method('isMounted') + $info->method('isMounted') ->willReturn($mounted); - $info->expects($this->any()) - ->method('getType') + $info->method('getType') ->willReturn($type); + $info->method('getInternalPath') + ->willReturn($internalPath); + $info->method('getMountPoint') + ->willReturnCallback(function() use ($shared) { + if ($shared) { + return $this->createMock(SharedMount::class); + } else { + return $this->createMock(MountPoint::class); + } + }); + $storage = $this->createMock(Storage\IStorage::class); + if ($shared) { + $storage->method('instanceOfStorage') + ->willReturn(true); + $cache = $this->createMock(ICache::class); + $storage->method('getCache') + ->willReturn($cache); + $shareRootEntry = $this->createMock(ICacheEntry::class); + $cache->method('get') + ->willReturn($shareRootEntry); + $shareRootEntry->method('getPermissions') + ->willReturn($shareRootPermissions); + } else { + $storage->method('instanceOfStorage') + ->willReturn(false); + } + $info->method('getStorage') + ->willReturn($storage); $view = $this->getMockBuilder(View::class) ->disableOriginalConstructor() ->getMock(); @@ -256,7 +288,7 @@ public function testSanitizeMtime($mtime, $expected) { public function invalidSanitizeMtimeProvider() { return [ - [-1337], [0], ['abcdef'], ['-1337'], ['0'], [12321], [24 * 60 * 60 - 1] + [-1337], [0], ['abcdef'], ['-1337'], ['0'], [12321], [24 * 60 * 60 - 1], ]; } diff --git a/lib/public/Files/DavUtil.php b/lib/public/Files/DavUtil.php index 343f3c2ac0f39..2e7efdccd071b 100644 --- a/lib/public/Files/DavUtil.php +++ b/lib/public/Files/DavUtil.php @@ -32,6 +32,9 @@ namespace OCP\Files; +use OCP\Constants; +use OC\Files\Mount\MoveableMount; + /** * This class provides different helper functions related to WebDAV protocol * @@ -73,10 +76,21 @@ public static function getDavPermissions(FileInfo $info): string { $p .= 'D'; } if ($info->isUpdateable()) { - $p .= 'NV'; // Renameable, Moveable + $p .= 'NV'; // Renameable, Movable } + + // since we always add update permissions for the root of movable mounts + // we need to check the shared cache item directly to determine if it's writable + $storage = $info->getStorage(); + if ($info->getInternalPath() === '' && $info->getMountPoint() instanceof MoveableMount) { + $rootEntry = $storage->getCache()->get(''); + $isWritable = $rootEntry->getPermissions() & Constants::PERMISSION_UPDATE; + } else { + $isWritable = $info->isUpdateable(); + } + if ($info->getType() === FileInfo::TYPE_FILE) { - if ($info->isUpdateable()) { + if ($isWritable) { $p .= 'W'; } } else {