Skip to content

Commit ee29ab1

Browse files
icewind1991nickvergessen
icewind1991
authored and
nickvergessen
committed
fix share roots always being marked as writable
Signed-off-by: Robin Appelman <robin@icewind.nl>
1 parent b4e8feb commit ee29ab1

2 files changed

Lines changed: 70 additions & 24 deletions

File tree

‎apps/dav/tests/unit/Connector/Sabre/NodeTest.php‎

Lines changed: 54 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,20 @@
2525
* along with this program. If not, see <http://www.gnu.org/licenses/>
2626
*
2727
*/
28+
2829
namespace OCA\DAV\Tests\unit\Connector\Sabre;
2930

3031
use OC\Files\FileInfo;
32+
use OC\Files\Mount\MountPoint;
3133
use OC\Files\View;
3234
use OC\Share20\ShareAttributes;
35+
use OCA\Files_Sharing\SharedMount;
3336
use OCA\Files_Sharing\SharedStorage;
37+
use OCP\Constants;
38+
use OCP\Files\Cache\ICacheEntry;
3439
use OCP\Files\Mount\IMountPoint;
3540
use OCP\Files\Storage;
41+
use OCP\ICache;
3642
use OCP\Share\IAttributes;
3743
use OCP\Share\IManager;
3844
use OCP\Share\IShare;
@@ -46,40 +52,66 @@
4652
class NodeTest extends \Test\TestCase {
4753
public function davPermissionsProvider() {
4854
return [
49-
[\OCP\Constants::PERMISSION_ALL, 'file', false, false, 'RGDNVW'],
50-
[\OCP\Constants::PERMISSION_ALL, 'dir', false, false, 'RGDNVCK'],
51-
[\OCP\Constants::PERMISSION_ALL, 'file', true, false, 'SRGDNVW'],
52-
[\OCP\Constants::PERMISSION_ALL, 'file', true, true, 'SRMGDNVW'],
53-
[\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_SHARE, 'file', true, false, 'SGDNVW'],
54-
[\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_UPDATE, 'file', false, false, 'RGD'],
55-
[\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_DELETE, 'file', false, false, 'RGNVW'],
56-
[\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_CREATE, 'file', false, false, 'RGDNVW'],
57-
[\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_READ, 'file', false, false, 'RDNVW'],
58-
[\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_CREATE, 'dir', false, false, 'RGDNV'],
59-
[\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_READ, 'dir', false, false, 'RDNVCK'],
55+
[Constants::PERMISSION_ALL, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGDNVW'],
56+
[Constants::PERMISSION_ALL, 'dir', false, Constants::PERMISSION_ALL, false, 'test', 'RGDNVCK'],
57+
[Constants::PERMISSION_ALL, 'file', true, Constants::PERMISSION_ALL, false, 'test', 'SRGDNVW'],
58+
[Constants::PERMISSION_ALL, 'file', true, Constants::PERMISSION_ALL, true, 'test', 'SRMGDNVW'],
59+
[Constants::PERMISSION_ALL, 'file', true, Constants::PERMISSION_ALL, true, '' , 'SRMGDNVW'],
60+
[Constants::PERMISSION_ALL, 'file', true, Constants::PERMISSION_ALL - Constants::PERMISSION_UPDATE, true, '' , 'SRMGDNV'],
61+
[Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE, 'file', true, Constants::PERMISSION_ALL, false, 'test', 'SGDNVW'],
62+
[Constants::PERMISSION_ALL - Constants::PERMISSION_UPDATE, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGD'],
63+
[Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGNVW'],
64+
[Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RGDNVW'],
65+
[Constants::PERMISSION_ALL - Constants::PERMISSION_READ, 'file', false, Constants::PERMISSION_ALL, false, 'test', 'RDNVW'],
66+
[Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE, 'dir', false, Constants::PERMISSION_ALL, false, 'test', 'RGDNV'],
67+
[Constants::PERMISSION_ALL - Constants::PERMISSION_READ, 'dir', false, Constants::PERMISSION_ALL, false, 'test', 'RDNVCK'],
6068
];
6169
}
6270

6371
/**
6472
* @dataProvider davPermissionsProvider
6573
*/
66-
public function testDavPermissions($permissions, $type, $shared, $mounted, $expected) {
74+
public function testDavPermissions($permissions, $type, $shared, $shareRootPermissions, $mounted, $internalPath, $expected) {
6775
$info = $this->getMockBuilder(FileInfo::class)
6876
->disableOriginalConstructor()
69-
->setMethods(['getPermissions', 'isShared', 'isMounted', 'getType'])
77+
->onlyMethods(['getPermissions', 'isShared', 'isMounted', 'getType', 'getInternalPath', 'getStorage', 'getMountPoint'])
7078
->getMock();
71-
$info->expects($this->any())
72-
->method('getPermissions')
79+
$info->method('getPermissions')
7380
->willReturn($permissions);
74-
$info->expects($this->any())
75-
->method('isShared')
81+
$info->method('isShared')
7682
->willReturn($shared);
77-
$info->expects($this->any())
78-
->method('isMounted')
83+
$info->method('isMounted')
7984
->willReturn($mounted);
80-
$info->expects($this->any())
81-
->method('getType')
85+
$info->method('getType')
8286
->willReturn($type);
87+
$info->method('getInternalPath')
88+
->willReturn($internalPath);
89+
$info->method('getMountPoint')
90+
->willReturnCallback(function() use ($shared) {
91+
if ($shared) {
92+
return $this->createMock(SharedMount::class);
93+
} else {
94+
return $this->createMock(MountPoint::class);
95+
}
96+
});
97+
$storage = $this->createMock(Storage\IStorage::class);
98+
if ($shared) {
99+
$storage->method('instanceOfStorage')
100+
->willReturn(true);
101+
$cache = $this->createMock(ICache::class);
102+
$storage->method('getCache')
103+
->willReturn($cache);
104+
$shareRootEntry = $this->createMock(ICacheEntry::class);
105+
$cache->method('get')
106+
->willReturn($shareRootEntry);
107+
$shareRootEntry->method('getPermissions')
108+
->willReturn($shareRootPermissions);
109+
} else {
110+
$storage->method('instanceOfStorage')
111+
->willReturn(false);
112+
}
113+
$info->method('getStorage')
114+
->willReturn($storage);
83115
$view = $this->getMockBuilder(View::class)
84116
->disableOriginalConstructor()
85117
->getMock();
@@ -256,7 +288,7 @@ public function testSanitizeMtime($mtime, $expected) {
256288

257289
public function invalidSanitizeMtimeProvider() {
258290
return [
259-
[-1337], [0], ['abcdef'], ['-1337'], ['0'], [12321], [24 * 60 * 60 - 1]
291+
[-1337], [0], ['abcdef'], ['-1337'], ['0'], [12321], [24 * 60 * 60 - 1],
260292
];
261293
}
262294

‎lib/public/Files/DavUtil.php‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232

3333
namespace OCP\Files;
3434

35+
use OCP\Constants;
36+
use OC\Files\Mount\MoveableMount;
37+
3538
/**
3639
* This class provides different helper functions related to WebDAV protocol
3740
*
@@ -73,10 +76,21 @@ public static function getDavPermissions(FileInfo $info): string {
7376
$p .= 'D';
7477
}
7578
if ($info->isUpdateable()) {
76-
$p .= 'NV'; // Renameable, Moveable
79+
$p .= 'NV'; // Renameable, Movable
7780
}
81+
82+
// since we always add update permissions for the root of movable mounts
83+
// we need to check the shared cache item directly to determine if it's writable
84+
$storage = $info->getStorage();
85+
if ($info->getInternalPath() === '' && $info->getMountPoint() instanceof MoveableMount) {
86+
$rootEntry = $storage->getCache()->get('');
87+
$isWritable = $rootEntry->getPermissions() & Constants::PERMISSION_UPDATE;
88+
} else {
89+
$isWritable = $info->isUpdateable();
90+
}
91+
7892
if ($info->getType() === FileInfo::TYPE_FILE) {
79-
if ($info->isUpdateable()) {
93+
if ($isWritable) {
8094
$p .= 'W';
8195
}
8296
} else {

0 commit comments

Comments
 (0)