sascha
/
nextcloud-server
mirror of https://github.com/nextcloud/server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(sharing): Move permission validation to share manager 

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
pull/50506/head
Ferdinand Thiessen 2025-01-21 11:59:22 +07:00
parent ee5f5bc159
commit dd5ce9eaa4
No known key found for this signature in database
GPG Key ID: 45FAE7268762B400
2 changed files with 36 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
lib/private/Share20/Manager.php
Unescape Escape View File

@ -212,6 +212,17 @@ class Manager implements IManager {
throw new \InvalidArgumentException('A share requires permissions');
}
// Permissions must be valid
if ($share->getPermissions() < 0 || $share->getPermissions() > \OCP\Constants::PERMISSION_ALL) {
throw new \InvalidArgumentException($this->l->t('Valid permissions are required for sharing'));
}
// Single file shares should never have delete or create permissions
if (($share->getNode() instanceof File)
&& (($share->getPermissions() & (\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_DELETE)) !== 0)) {
throw new \InvalidArgumentException($this->l->t('File shares cannot have create or delete permissions'));
}
$permissions = 0;
$nodesForUser = $userFolder->getById($share->getNodeId());
foreach ($nodesForUser as $node) {

30
tests/lib/Share20/ManagerTest.php
Unescape Escape View File

@ -885,10 +885,9 @@ class ManagerTest extends \Test\TestCase {
$mount = $this->createMock(MoveableMount::class);
$limitedPermssions->method('getMountPoint')->willReturn($mount);
$data[] = [$this->createShare(null, IShare::TYPE_USER, $limitedPermssions, $user2, $user0, $user0, 31, null, null), 'Cannot increase permissions of path', true];
// increase permissions of a re-share
$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $limitedPermssions, $group0, $user0, $user0, 17, null, null), 'Cannot increase permissions of path', true];
$data[] = [$this->createShare(null, IShare::TYPE_LINK, $limitedPermssions, null, $user0, $user0, 3, null, null), 'Cannot increase permissions of path', true];
$data[] = [$this->createShare(null, IShare::TYPE_USER, $limitedPermssions, $user2, $user0, $user0, 3, null, null), 'Cannot increase permissions of path', true];
$nonMovableStorage = $this->createMock(Storage\IStorage::class);
$nonMovableStorage->method('instanceOfStorage')
@ -919,6 +918,20 @@ class ManagerTest extends \Test\TestCase {
$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $rootFolder, $group0, $user0, $user0, 2, null, null), 'You cannot share your root folder', true];
$data[] = [$this->createShare(null, IShare::TYPE_LINK, $rootFolder, null, $user0, $user0, 16, null, null), 'You cannot share your root folder', true];
$allPermssionsFiles = $this->createMock(File::class);
$allPermssionsFiles->method('isShareable')->willReturn(true);
$allPermssionsFiles->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_ALL);
$allPermssionsFiles->method('getId')->willReturn(187);
$allPermssionsFiles->method('getOwner')
->willReturn($owner);
$allPermssionsFiles->method('getStorage')
->willReturn($storage);
// test invalid CREATE or DELETE permissions
$data[] = [$this->createShare(null, IShare::TYPE_USER, $allPermssionsFiles, $user2, $user0, $user0, \OCP\Constants::PERMISSION_ALL, null, null), 'File shares cannot have create or delete permissions', true];
$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $allPermssionsFiles, $group0, $user0, $user0, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE, null, null), 'File shares cannot have create or delete permissions', true];
$data[] = [$this->createShare(null, IShare::TYPE_LINK, $allPermssionsFiles, null, $user0, $user0, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_DELETE, null, null), 'File shares cannot have create or delete permissions', true];
$allPermssions = $this->createMock(Folder::class);
$allPermssions->method('isShareable')->willReturn(true);
$allPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_ALL);
@ -931,6 +944,12 @@ class ManagerTest extends \Test\TestCase {
$data[] = [$this->createShare(null, IShare::TYPE_USER, $allPermssions, $user2, $user0, $user0, 30, null, null), 'Shares need at least read permissions', true];
$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $allPermssions, $group0, $user0, $user0, 2, null, null), 'Shares need at least read permissions', true];
// test invalid permissions
$data[] = [$this->createShare(null, IShare::TYPE_USER, $allPermssions, $user2, $user0, $user0, 32, null, null), 'Valid permissions are required for sharing', true];
$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $allPermssions, $group0, $user0, $user0, 63, null, null), 'Valid permissions are required for sharing', true];
$data[] = [$this->createShare(null, IShare::TYPE_LINK, $allPermssions, null, $user0, $user0, -1, null, null), 'Valid permissions are required for sharing', true];
// working shares
$data[] = [$this->createShare(null, IShare::TYPE_USER, $allPermssions, $user2, $user0, $user0, 31, null, null), null, false];
$data[] = [$this->createShare(null, IShare::TYPE_GROUP, $allPermssions, $group0, $user0, $user0, 3, null, null), null, false];
$data[] = [$this->createShare(null, IShare::TYPE_LINK, $allPermssions, null, $user0, $user0, 17, null, null), null, false];
@ -2397,9 +2416,10 @@ class ManagerTest extends \Test\TestCase {
$this->assertEquals($expected, !$exception);
}
public function testCreateShareUser() {
public function testCreateShareUser(): void {
/** @var Manager|MockObject $manager */
$manager = $this->createManagerMock()
->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks'])
->onlyMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks'])
->getMock();
$shareOwner = $this->createMock(IUser::class);