diff --git a/apps/files_sharing/tests/EtagPropagationTest.php b/apps/files_sharing/tests/EtagPropagationTest.php index ee2f5b204e..7ba01b737e 100644 --- a/apps/files_sharing/tests/EtagPropagationTest.php +++ b/apps/files_sharing/tests/EtagPropagationTest.php @@ -128,6 +128,8 @@ class EtagPropagationTest extends PropagationTestCase { $view2 = new View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files'); $view2->mkdir('/sub1/sub2'); $view2->rename('/folder', '/sub1/sub2/folder'); + $this->loginAsUser(self::TEST_FILES_SHARING_API_USER2); + $insideInfo = $view2->getFileInfo('/sub1/sub2/folder/inside'); $this->assertInstanceOf('\OC\Files\FileInfo', $insideInfo); diff --git a/apps/files_sharing/tests/TestCase.php b/apps/files_sharing/tests/TestCase.php index a201974de4..34924110b3 100644 --- a/apps/files_sharing/tests/TestCase.php +++ b/apps/files_sharing/tests/TestCase.php @@ -125,6 +125,14 @@ abstract class TestCase extends \Test\TestCase { $qb->delete('share'); $qb->execute(); + $qb = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $qb->delete('mounts'); + $qb->execute(); + + $qb = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $qb->delete('filecache'); + $qb->execute(); + parent::tearDown(); } diff --git a/build/integration/sharing_features/sharing-v1-part2.feature b/build/integration/sharing_features/sharing-v1-part2.feature index 52fbe73dc9..8fc06fbdde 100644 --- a/build/integration/sharing_features/sharing-v1-part2.feature +++ b/build/integration/sharing_features/sharing-v1-part2.feature @@ -562,6 +562,36 @@ Feature: sharing Then the OCS status code should be "404" And the HTTP status code should be "200" + Scenario: User is allowed to reshare file with more permissions if shares of same file to same user have them + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And group "group1" exists + And user "user1" belongs to group "group1" + And As an "user0" + And creating a share with + | path | /textfile0.txt | + | shareType | 1 | + | shareWith | group1 | + | permissions | 15 | + And As an "user1" + And accepting last share + And As an "user0" + And creating a share with + | path | /textfile0.txt | + | shareType | 0 | + | shareWith | user1 | + | permissions | 17 | + And As an "user1" + And accepting last share + When creating a share with + | path | /textfile0 (2).txt | + | shareType | 0 | + | shareWith | user2 | + | permissions | 19 | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + Scenario: User is not allowed to reshare file with more permissions As an "admin" Given user "user0" exists @@ -583,6 +613,92 @@ Feature: sharing Then the OCS status code should be "404" And the HTTP status code should be "200" + Scenario: User is not allowed to reshare file with more permissions even if shares of same file to other users have them + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And user "user3" exists + And As an "user0" + And creating a share with + | path | /textfile0.txt | + | shareType | 0 | + | shareWith | user3 | + | permissions | 15 | + And As an "user3" + And accepting last share + And As an "user0" + And creating a share with + | path | /textfile0.txt | + | shareType | 0 | + | shareWith | user1 | + | permissions | 17 | + And As an "user1" + And accepting last share + When creating a share with + | path | /textfile0 (2).txt | + | shareType | 0 | + | shareWith | user2 | + | permissions | 19 | + Then the OCS status code should be "404" + And the HTTP status code should be "200" + + Scenario: User is not allowed to reshare file with more permissions even if shares of other files from same user have them + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And As an "user0" + And creating a share with + | path | /textfile0.txt | + | shareType | 0 | + | shareWith | user1 | + | permissions | 15 | + And As an "user1" + And accepting last share + And As an "user0" + And creating a share with + | path | /textfile1.txt | + | shareType | 0 | + | shareWith | user1 | + | permissions | 17 | + And As an "user1" + And accepting last share + When creating a share with + | path | /textfile1 (2).txt | + | shareType | 0 | + | shareWith | user2 | + | permissions | 19 | + Then the OCS status code should be "404" + And the HTTP status code should be "200" + + Scenario: User is not allowed to reshare file with more permissions even if shares of other files from other users have them + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And user "user3" exists + And As an "user3" + And creating a share with + | path | /textfile0.txt | + | shareType | 0 | + | shareWith | user1 | + | permissions | 15 | + And As an "user1" + And accepting last share + And As an "user0" + And creating a share with + | path | /textfile1.txt | + | shareType | 0 | + | shareWith | user1 | + | permissions | 17 | + And As an "user1" + And accepting last share + When creating a share with + | path | /textfile1 (2).txt | + | shareType | 0 | + | shareWith | user2 | + | permissions | 19 | + Then the OCS status code should be "404" + And the HTTP status code should be "200" + Scenario: User is not allowed to reshare file with additional delete permissions As an "admin" Given user "user0" exists @@ -857,6 +973,39 @@ Feature: sharing Then the OCS status code should be "100" And the HTTP status code should be "200" + Scenario: Allow reshare to exceed permissions if shares of same file to same user have them + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And group "group1" exists + And user "user1" belongs to group "group1" + And user "user0" created a folder "/TMP" + And As an "user0" + And creating a share with + | path | /TMP | + | shareType | 1 | + | shareWith | group1 | + | permissions | 15 | + And As an "user1" + And accepting last share + And As an "user0" + And creating a share with + | path | /TMP | + | shareType | 0 | + | shareWith | user1 | + | permissions | 17 | + And As an "user1" + And accepting last share + And creating a share with + | path | /TMP | + | shareType | 0 | + | shareWith | user2 | + | permissions | 17 | + When Updating last share with + | permissions | 31 | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + Scenario: Do not allow reshare to exceed permissions Given user "user0" exists And user "user1" exists @@ -880,6 +1029,101 @@ Feature: sharing Then the OCS status code should be "404" And the HTTP status code should be "200" + Scenario: Do not allow reshare to exceed permissions even if shares of same file to other users have them + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And user "user3" exists + And user "user0" created a folder "/TMP" + And As an "user0" + And creating a share with + | path | /TMP | + | shareType | 0 | + | shareWith | user3 | + | permissions | 15 | + And As an "user3" + And accepting last share + And As an "user0" + And creating a share with + | path | /TMP | + | shareType | 0 | + | shareWith | user1 | + | permissions | 21 | + And As an "user1" + And accepting last share + And creating a share with + | path | /TMP | + | shareType | 0 | + | shareWith | user2 | + | permissions | 21 | + When Updating last share with + | permissions | 31 | + Then the OCS status code should be "404" + And the HTTP status code should be "200" + + Scenario: Do not allow reshare to exceed permissions even if shares of other files from same user have them + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And As an "user0" + And creating a share with + | path | /FOLDER | + | shareType | 0 | + | shareWith | user1 | + | permissions | 15 | + And As an "user1" + And accepting last share + And user "user0" created a folder "/TMP" + And As an "user0" + And creating a share with + | path | /TMP | + | shareType | 0 | + | shareWith | user1 | + | permissions | 21 | + And As an "user1" + And accepting last share + And creating a share with + | path | /TMP | + | shareType | 0 | + | shareWith | user2 | + | permissions | 21 | + When Updating last share with + | permissions | 31 | + Then the OCS status code should be "404" + And the HTTP status code should be "200" + + Scenario: Do not allow reshare to exceed permissions even if shares of other files from other users have them + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And user "user3" exists + And As an "user3" + And creating a share with + | path | /FOLDER | + | shareType | 0 | + | shareWith | user1 | + | permissions | 15 | + And As an "user1" + And accepting last share + And user "user0" created a folder "/TMP" + And As an "user0" + And creating a share with + | path | /TMP | + | shareType | 0 | + | shareWith | user1 | + | permissions | 21 | + And As an "user1" + And accepting last share + And creating a share with + | path | /TMP | + | shareType | 0 | + | shareWith | user2 | + | permissions | 21 | + When Updating last share with + | permissions | 31 | + Then the OCS status code should be "404" + And the HTTP status code should be "200" + Scenario: Do not allow sub reshare to exceed permissions Given user "user0" exists And user "user1" exists diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php index d3c4e1afc6..9d534815cd 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -437,6 +437,10 @@ class Filesystem { // home mounts are handled seperate since we need to ensure this is mounted before we call the other mount providers $homeMount = $mountConfigManager->getHomeMountForUser($userObject); + if ($homeMount->getStorageRootId() === -1) { + $homeMount->getStorage()->mkdir(''); + $homeMount->getStorage()->getScanner()->scan(''); + } self::getMountManager()->addMount($homeMount); diff --git a/lib/private/Files/Mount/LocalHomeMountProvider.php b/lib/private/Files/Mount/LocalHomeMountProvider.php index 744a57981e..68c69cc8cd 100644 --- a/lib/private/Files/Mount/LocalHomeMountProvider.php +++ b/lib/private/Files/Mount/LocalHomeMountProvider.php @@ -35,7 +35,7 @@ class LocalHomeMountProvider implements IHomeMountProvider { * * @param IUser $user * @param IStorageFactory $loader - * @return \OCP\Files\Mount\IMountPoint[] + * @return \OCP\Files\Mount\IMountPoint|null */ public function getHomeMountForUser(IUser $user, IStorageFactory $loader) { $arguments = ['user' => $user]; diff --git a/lib/private/Files/Mount/MountPoint.php b/lib/private/Files/Mount/MountPoint.php index 2cb25f0704..f9cda6fbce 100644 --- a/lib/private/Files/Mount/MountPoint.php +++ b/lib/private/Files/Mount/MountPoint.php @@ -268,7 +268,7 @@ class MountPoint implements IMountPoint { * @return int */ public function getStorageRootId() { - if (is_null($this->rootId)) { + if (is_null($this->rootId) || $this->rootId === -1) { $this->rootId = (int)$this->getStorage()->getCache()->getId(''); } return $this->rootId; diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 24aa54ea0d..3b022f5951 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -298,13 +298,20 @@ class Manager implements IManager { $isFederatedShare = $share->getNode()->getStorage()->instanceOfStorage('\OCA\Files_Sharing\External\Storage'); $permissions = 0; - $mount = $share->getNode()->getMountPoint(); + + $userMounts = $userFolder->getById($share->getNode()->getId()); + $userMount = array_shift($userMounts); + $mount = $userMount->getMountPoint(); if (!$isFederatedShare && $share->getNode()->getOwner() && $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); $userMountPoint = array_shift($userMountPoints); + if ($userMountPoint === null) { + throw new GenericShareException('Could not get proper user mount for ' . $userMountPointId . '. Failing since else the next calls are called with null'); + } + /* Check if this is an incoming share */ $incomingShares = $this->getSharedWith($share->getSharedBy(), IShare::TYPE_USER, $userMountPoint, -1, 0); $incomingShares = array_merge($incomingShares, $this->getSharedWith($share->getSharedBy(), IShare::TYPE_GROUP, $userMountPoint, -1, 0)); diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 65a5ff8946..58f96c418b 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -613,6 +613,7 @@ class ManagerTest extends \Test\TestCase { $limitedPermssions = $this->createMock(File::class); $limitedPermssions->method('isShareable')->willReturn(true); $limitedPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ); + $limitedPermssions->method('getId')->willReturn(108); $limitedPermssions->method('getPath')->willReturn('path'); $limitedPermssions->method('getOwner') ->willReturn($owner); @@ -634,6 +635,7 @@ class ManagerTest extends \Test\TestCase { $nonMoveableMountPermssions = $this->createMock(Folder::class); $nonMoveableMountPermssions->method('isShareable')->willReturn(true); $nonMoveableMountPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ); + $nonMoveableMountPermssions->method('getId')->willReturn(108); $nonMoveableMountPermssions->method('getPath')->willReturn('path'); $nonMoveableMountPermssions->method('getOwner') ->willReturn($owner); @@ -655,6 +657,7 @@ class ManagerTest extends \Test\TestCase { $allPermssions = $this->createMock(Folder::class); $allPermssions->method('isShareable')->willReturn(true); $allPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_ALL); + $allPermssions->method('getId')->willReturn(108); $allPermssions->method('getOwner') ->willReturn($owner); $allPermssions->method('getStorage') @@ -675,6 +678,7 @@ class ManagerTest extends \Test\TestCase { $remoteFile = $this->createMock(Folder::class); $remoteFile->method('isShareable')->willReturn(true); $remoteFile->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ ^ \OCP\Constants::PERMISSION_UPDATE); + $remoteFile->method('getId')->willReturn(108); $remoteFile->method('getOwner') ->willReturn($owner); $remoteFile->method('getStorage') @@ -709,6 +713,11 @@ class ManagerTest extends \Test\TestCase { $userFolder->expects($this->any()) ->method('getId') ->willReturn(42); + // Id 108 is used in the data to refer to the node of the share. + $userFolder->expects($this->any()) + ->method('getById') + ->with(108) + ->willReturn([$share->getNode()]); $userFolder->expects($this->any()) ->method('getRelativePath') ->willReturnArgument(0); diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index b28be47875..88c5b46854 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -441,7 +441,7 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { } } - private function IsDatabaseAccessAllowed() { + protected function IsDatabaseAccessAllowed() { // on travis-ci.org we allow database access in any case - otherwise // this will break all apps right away if (true == getenv('TRAVIS')) { diff --git a/tests/lib/Traits/MountProviderTrait.php b/tests/lib/Traits/MountProviderTrait.php index 0437157e84..379d33ea71 100644 --- a/tests/lib/Traits/MountProviderTrait.php +++ b/tests/lib/Traits/MountProviderTrait.php @@ -33,6 +33,12 @@ trait MountProviderTrait { $this->mounts[$userId] = []; } $this->mounts[$userId][] = ['storage' => $storage, 'mountPoint' => $mountPoint, 'arguments' => $arguments]; + + if ($this->IsDatabaseAccessAllowed()) { + $mount = new MountPoint($storage, $mountPoint, $arguments, $this->storageFactory); + $storage = $mount->getStorage(); + $storage->getScanner()->scan(''); + } } protected function registerStorageWrapper($name, $wrapper) {