upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-05-28 04:32:30 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 54

fix(sharing): Move permission validation to share manager

Browse source 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
This commit is contained in:
Ferdinand Thiessen 2025-01-21 11:59:22 +01:00
parent 253f4345f1
commit 0a0f56282a
No known key found for this signature in database
GPG key ID: 45FAE7268762B400
2 changed files with 35 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
lib/private/Share20/Manager.php
View file

@ -217,6 +217,17 @@ class Manager implements IManager {
throw new \InvalidArgumentException($this->l->t('Valid permissions are required for sharing'));
}
// 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) {

28
tests/lib/Share20/ManagerTest.php
View file

@ -893,10 +893,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(IStorage::class);
$nonMovableStorage->method('instanceOfStorage')
@ -927,6 +926,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);
@ -939,6 +952,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];
@ -2406,8 +2425,9 @@ class ManagerTest extends \Test\TestCase {
}
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);