Fix permission check on incoming federated shares

Since federated shares have their permissions set on the node, we do not need
to check for parent share permissions. Otherwise reshares of incoming federated
have no permission variable defined and creating them will fail

Signed-off-by: Julius Härtl <jus@bitgrid.net>
pull/16592/head
Julius Härtl 7 years ago
parent f1066fd769
commit 22b81ac1e4
No known key found for this signature in database
GPG Key ID: 4C614C6ED2CDE6DF
  1. 5
      lib/private/Share20/Manager.php
  2. 33
      tests/lib/Share20/ManagerTest.php

@ -290,8 +290,10 @@ class Manager implements IManager {
throw new \InvalidArgumentException('A share requires permissions');
}
$isFederatedShare = $share->getNode()->getStorage()->instanceOfStorage('\OCA\Files_Sharing\External\Storage');
$permissions = 0;
$mount = $share->getNode()->getMountPoint();
if ($share->getNode()->getOwner()->getUID() !== $share->getSharedBy()) {
if (!$isFederatedShare && $share->getNode()->getOwner()->getUID() !== $share->getSharedBy()) {
// When it's a reshare use the parent share permissions as maximum
$userMountPointId = $mount->getStorageRootId();
$userMountPoints = $userFolder->getById($userMountPointId);
@ -304,7 +306,6 @@ class Manager implements IManager {
/** @var \OCP\Share\IShare[] $incomingShares */
if (!empty($incomingShares)) {
$permissions = 0;
foreach ($incomingShares as $incomingShare) {
$permissions |= $incomingShare->getPermissions();
}

@ -551,6 +551,14 @@ class ManagerTest extends \Test\TestCase {
$file = $this->createMock(File::class);
$node = $this->createMock(Node::class);
$storage = $this->createMock(Storage\IStorage::class);
$storage->method('instanceOfStorage')
->with('\OCA\Files_Sharing\External\Storage')
->willReturn(false);
$file->method('getStorage')
->willReturn($storage);
$node->method('getStorage')
->willReturn($storage);
$data = [
[$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $file, null, $user0, $user0, 31, null, null), 'SharedWith is not a valid user', true],
@ -584,6 +592,8 @@ class ManagerTest extends \Test\TestCase {
$nonShareAble->method('getPath')->willReturn('path');
$nonShareAble->method('getOwner')
->willReturn($owner);
$nonShareAble->method('getStorage')
->willReturn($storage);
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $nonShareAble, $user2, $user0, $user0, 31, null, null), 'You are not allowed to share path', true];
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $nonShareAble, $group0, $user0, $user0, 31, null, null), 'You are not allowed to share path', true];
@ -595,6 +605,8 @@ class ManagerTest extends \Test\TestCase {
$limitedPermssions->method('getPath')->willReturn('path');
$limitedPermssions->method('getOwner')
->willReturn($owner);
$limitedPermssions->method('getStorage')
->willReturn($storage);
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $limitedPermssions, $user2, $user0, $user0, null, null, null), 'A share requires permissions', true];
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $limitedPermssions, $group0, $user0, $user0, null, null, null), 'A share requires permissions', true];
@ -603,6 +615,7 @@ class ManagerTest extends \Test\TestCase {
$mount = $this->createMock(MoveableMount::class);
$limitedPermssions->method('getMountPoint')->willReturn($mount);
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $limitedPermssions, $user2, $user0, $user0, 31, null, null), 'Can’t increase permissions of path', true];
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $limitedPermssions, $group0, $user0, $user0, 17, null, null), 'Can’t increase permissions of path', true];
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_LINK, $limitedPermssions, null, $user0, $user0, 3, null, null), 'Can’t increase permissions of path', true];
@ -613,6 +626,8 @@ class ManagerTest extends \Test\TestCase {
$nonMoveableMountPermssions->method('getPath')->willReturn('path');
$nonMoveableMountPermssions->method('getOwner')
->willReturn($owner);
$nonMoveableMountPermssions->method('getStorage')
->willReturn($storage);
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $nonMoveableMountPermssions, $user2, $user0, $user0, 11, null, null), 'Can’t increase permissions of path', false];
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $nonMoveableMountPermssions, $group0, $user0, $user0, 11, null, null), 'Can’t increase permissions of path', false];
@ -631,6 +646,8 @@ class ManagerTest extends \Test\TestCase {
$allPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_ALL);
$allPermssions->method('getOwner')
->willReturn($owner);
$allPermssions->method('getStorage')
->willReturn($storage);
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $allPermssions, $user2, $user0, $user0, 30, null, null), 'Shares need at least read permissions', true];
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $allPermssions, $group0, $user0, $user0, 2, null, null), 'Shares need at least read permissions', true];
@ -639,6 +656,22 @@ class ManagerTest extends \Test\TestCase {
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $allPermssions, $group0, $user0, $user0, 3, null, null), null, false];
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_LINK, $allPermssions, null, $user0, $user0, 17, null, null), null, false];
$remoteStorage = $this->createMock(Storage\IStorage::class);
$remoteStorage->method('instanceOfStorage')
->with('\OCA\Files_Sharing\External\Storage')
->willReturn(true);
$remoteFile = $this->createMock(Folder::class);
$remoteFile->method('isShareable')->willReturn(true);
$remoteFile->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ ^ \OCP\Constants::PERMISSION_UPDATE);
$remoteFile->method('getOwner')
->willReturn($owner);
$remoteFile->method('getStorage')
->willReturn($storage);
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_REMOTE, $remoteFile, $user2, $user0, $user0, 1, null, null), null, false];
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_REMOTE, $remoteFile, $user2, $user0, $user0, 3, null, null), null, false];
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_REMOTE, $remoteFile, $user2, $user0, $user0, 31, null, null), 'Can’t increase permissions of ', true];
return $data;
}

Loading…
Cancel
Save