fix share roots always being marked as writable

Signed-off-by: Robin Appelman <robin@icewind.nl>

diff --git a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php
index 751e4c138b2564fa91324d7fa5dea1cfff538ad8..83765d338f2c739f763ad053d4d92a1a2e0e5f58 100644 (file)
--- 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 <http://www.gnu.org/licenses/>
  *
  */
+
 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 @@ use OCP\Share\IShare;
 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): void {
+       public function testDavPermissions($permissions, $type, $shared, $shareRootPermissions, $mounted, $internalPath, $expected): void {
                $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 @@ class NodeTest extends \Test\TestCase {
 
        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 343f3c2ac0f39efe154a64bd032a88ec6da57045..2e7efdccd071be19e33f16ce7969e5d2d750a2d5 100644 (file)
--- 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 @@ class DavUtil {
                        $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 {