mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #16127 from nextcloud/backport/16097/stable16 

[stable16] Better check reshare permissions
This commit is contained in:
Roeland Jago Douma 2019-06-27 22:10:16 +02:00 committed by GitHub
parent 66dc4c9c72 808280b1a6
commit 0f8a3e8fc2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 291 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
apps/files_sharing/lib/Controller/ShareAPIController.php
View File

@ -975,10 +975,20 @@ class ShareAPIController extends OCSController {
}
if ($permissions !== null && $share->getShareOwner() !== $this->currentUser) {
// Get the root mount point for the user and check the share permissions there
$userFolder = $this->rootFolder->getUserFolder($this->currentUser);
$userNodes = $userFolder->getById($share->getNodeId());
$userNode = array_shift($userNodes);
$userMountPointId = $userNode->getMountPoint()->getStorageRootId();
$userMountPoints = $userFolder->getById($userMountPointId);
$userMountPoint = array_shift($userMountPoints);
/* Check if this is an incoming share */
$incomingShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_USER, $share->getNode(), -1, 0);
$incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_GROUP, $share->getNode(), -1, 0));
$incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_ROOM, $share->getNode(), -1, 0));
$incomingShares = $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_USER, $userMountPoint, -1, 0);
$incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_GROUP, $userMountPoint, -1, 0));
$incomingShares = array_merge($incomingShares, $this->shareManager->getSharedWith($this->currentUser, Share::SHARE_TYPE_ROOM, $userMountPoint, -1, 0));
/** @var \OCP\Share\IShare[] $incomingShares */
if (!empty($incomingShares)) {

230
apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
View File

@ -31,6 +31,7 @@ use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\OCS\OCSNotFoundException;
use OCP\Files\File;
use OCP\Files\Folder;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\Storage;
use OCP\IConfig;
use OCP\IL10N;
@ -1573,6 +1574,8 @@ class ShareAPIControllerTest extends TestCase {
$ocs = $this->mockFormatShare();
$node = $this->getMockBuilder(Folder::class)->getMock();
$node->method('getId')
->willReturn(42);
$share = $this->newShare();
$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
->setSharedBy($this->currentUser)
@ -1607,6 +1610,21 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager->method('getSharedWith')
->willReturn([]);
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$node]);
$mountPoint = $this->createMock(IMountPoint::class);
$node->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$expected = new DataResponse([]);
$result = $ocs->updateShare(42, null, '', null, 'false', '', '', '', 'false');
@ -1618,6 +1636,8 @@ class ShareAPIControllerTest extends TestCase {
$ocs = $this->mockFormatShare();
$folder = $this->getMockBuilder(Folder::class)->getMock();
$folder->method('getId')
->willReturn(42);
$share = \OC::$server->getShareManager()->newShare();
$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
@ -1645,6 +1665,21 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager->method('getSharedWith')
->willReturn([]);
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$folder]);
$mountPoint = $this->createMock(IMountPoint::class);
$folder->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$expected = new DataResponse([]);
$result = $ocs->updateShare(42, null, 'password', null, 'true', '2000-01-01', 'note', 'label', 'true');
@ -1659,6 +1694,8 @@ class ShareAPIControllerTest extends TestCase {
$ocs = $this->mockFormatShare();
$folder = $this->getMockBuilder(Folder::class)->getMock();
$folder->method('getId')
->willReturn(42);
$share = \OC::$server->getShareManager()->newShare();
$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
@ -1679,6 +1716,21 @@ class ShareAPIControllerTest extends TestCase {
})
)->will($this->returnArgument(0));
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$folder]);
$mountPoint = $this->createMock(IMountPoint::class);
$folder->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$expected = new DataResponse([]);
$result = $ocs->updateShare(42, $permissions, $password, null, $publicUpload, $expireDate);
@ -1949,6 +2001,9 @@ class ShareAPIControllerTest extends TestCase {
$date->setTime(0,0,0);
$node = $this->getMockBuilder(File::class)->getMock();
$node->method('getId')
->willReturn(42);
$share = $this->newShare();
$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
->setSharedBy($this->currentUser)
@ -1982,6 +2037,27 @@ class ShareAPIControllerTest extends TestCase {
})
)->will($this->returnArgument(0));
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$node]);
$mountPoint = $this->createMock(IMountPoint::class);
$node->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$mountPoint = $this->createMock(IMountPoint::class);
$node->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$expected = new DataResponse([]);
$result = $ocs->updateShare(42, null, null, 'false', null, null, null, null, null);
@ -1993,6 +2069,9 @@ class ShareAPIControllerTest extends TestCase {
$ocs = $this->mockFormatShare();
$node = $this->getMockBuilder(File::class)->getMock();
$node->method('getId')
->willReturn(42);
$share = $this->newShare();
$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
->setSharedBy($this->currentUser)
@ -2027,6 +2106,21 @@ class ShareAPIControllerTest extends TestCase {
})
)->will($this->returnArgument(0));
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$node]);
$mountPoint = $this->createMock(IMountPoint::class);
$node->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$expected = new DataResponse([]);
$result = $ocs->updateShare(42, null, null, null, null, '2010-12-23', null, null, null);
@ -2040,6 +2134,8 @@ class ShareAPIControllerTest extends TestCase {
$date = new \DateTime('2000-01-01');
$folder = $this->getMockBuilder(Folder::class)->getMock();
$folder->method('getId')
->willReturn(42);
$share = \OC::$server->getShareManager()->newShare();
$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
@ -2072,6 +2168,21 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager->method('getSharedWith')
->willReturn([]);
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$folder]);
$mountPoint = $this->createMock(IMountPoint::class);
$folder->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$expected = new DataResponse([]);
$result = $ocs->updateShare(42, null, null, null, 'true', null, null, null, null);
@ -2085,6 +2196,8 @@ class ShareAPIControllerTest extends TestCase {
$date = new \DateTime('2000-01-01');
$folder = $this->getMockBuilder(Folder::class)->getMock();
$folder->method('getId')
->willReturn(42);
$share = \OC::$server->getShareManager()->newShare();
$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
@ -2116,6 +2229,21 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager->method('getSharedWith')->willReturn([]);
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$folder]);
$mountPoint = $this->createMock(IMountPoint::class);
$folder->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$expected = new DataResponse([]);
$result = $ocs->updateShare(42, 7, null, null, null, null, null, null, null);
@ -2129,6 +2257,8 @@ class ShareAPIControllerTest extends TestCase {
$date = new \DateTime('2000-01-01');
$folder = $this->getMockBuilder(Folder::class)->getMock();
$folder->method('getId')
->willReturn(42);
$share = \OC::$server->getShareManager()->newShare();
$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
@ -2158,6 +2288,21 @@ class ShareAPIControllerTest extends TestCase {
})
)->will($this->returnArgument(0));
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$folder]);
$mountPoint = $this->createMock(IMountPoint::class);
$folder->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$this->shareManager->method('getSharedWith')->willReturn([]);
$expected = new DataResponse([]);
@ -2171,6 +2316,8 @@ class ShareAPIControllerTest extends TestCase {
$ocs = $this->mockFormatShare();
$file = $this->getMockBuilder(File::class)->getMock();
$file->method('getId')
->willReturn(42);
$share = \OC::$server->getShareManager()->newShare();
$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
@ -2189,6 +2336,21 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager->method('getSharedWith')->willReturn([]);
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$file]);
$mountPoint = $this->createMock(IMountPoint::class);
$file->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$expected = new DataResponse([]);
$result = $ocs->updateShare(42, 31, null, null, null, null);
@ -2200,6 +2362,8 @@ class ShareAPIControllerTest extends TestCase {
$ocs = $this->mockFormatShare();
$folder = $this->createMock(Folder::class);
$folder->method('getId')
->willReturn(42);
$share = \OC::$server->getShareManager()->newShare();
$share
@ -2239,6 +2403,21 @@ class ShareAPIControllerTest extends TestCase {
['currentUser', \OCP\Share::SHARE_TYPE_ROOM, $share->getNode(), -1, 0, []]
]));
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$folder]);
$mountPoint = $this->createMock(IMountPoint::class);
$folder->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$this->shareManager->expects($this->never())->method('updateShare');
try {
@ -2253,6 +2432,8 @@ class ShareAPIControllerTest extends TestCase {
$ocs = $this->mockFormatShare();
$folder = $this->createMock(Folder::class);
$folder->method('getId')
->willReturn(42);
$share = \OC::$server->getShareManager()->newShare();
$share
@ -2285,6 +2466,21 @@ class ShareAPIControllerTest extends TestCase {
['currentUser', \OCP\Share::SHARE_TYPE_ROOM, $share->getNode(), -1, 0, []]
]));
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$folder]);
$mountPoint = $this->createMock(IMountPoint::class);
$folder->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$this->shareManager->expects($this->never())->method('updateShare');
$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
@ -2300,6 +2496,8 @@ class ShareAPIControllerTest extends TestCase {
$ocs = $this->mockFormatShare();
$folder = $this->createMock(Folder::class);
$folder->method('getId')
->willReturn(42);
$share = \OC::$server->getShareManager()->newShare();
$share
@ -2341,6 +2539,21 @@ class ShareAPIControllerTest extends TestCase {
}
));
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$folder]);
$mountPoint = $this->createMock(IMountPoint::class);
$folder->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$this->shareManager->expects($this->any())
->method('getSharedWith')
->will($this->returnValueMap([
@ -2363,6 +2576,8 @@ class ShareAPIControllerTest extends TestCase {
$ocs = $this->mockFormatShare();
$folder = $this->createMock(Folder::class);
$folder->method('getId')
->willReturn(42);
$share = \OC::$server->getShareManager()->newShare();
$share
@ -2400,6 +2615,21 @@ class ShareAPIControllerTest extends TestCase {
->with($share)
->willReturn($share);
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')
->with($this->currentUser)
->willReturn($userFolder);
$userFolder->method('getById')
->with(42)
->willReturn([$folder]);
$mountPoint = $this->createMock(IMountPoint::class);
$folder->method('getMountPoint')
->willReturn($mountPoint);
$mountPoint->method('getStorageRootId')
->willReturn(42);
$result = $ocs->updateShare(42, 31);
$this->assertInstanceOf(DataResponse::class, $result);
}

22
build/integration/features/sharing-v1-part2.feature
View File

@ -417,6 +417,28 @@ Feature: sharing
| permissions | 31 |
Then the OCS status code should be "404"
Scenario: Do not allow sub reshare to exceed permissions
Given user "user0" exists
And user "user1" exists
And user "user2" exists
And user "user0" created a folder "/TMP"
And user "user0" created a folder "/TMP/SUB"
And As an "user0"
And creating a share with
| path | /TMP |
| shareType | 0 |
| shareWith | user1 |
| permissions | 21 |
And As an "user1"
And creating a share with
| path | /TMP/SUB |
| shareType | 0 |
| shareWith | user2 |
| permissions | 21 |
When Updating last share with
| permissions | 31 |
Then the OCS status code should be "404"
Scenario: Only allow 1 link share per file/folder
Given user "user0" exists
And As an "user0"

29
build/integration/features/sharing-v1-part3.feature
View File

@ -339,14 +339,16 @@ Feature: sharing
Scenario: do not allow to increase link share permissions on reshare
Given As an "admin"
And user "admin" created a folder "/TMP"
And user "user0" exists
And user "user1" exists
And user "user0" created a folder "/TMP"
And As an "user0"
And creating a share with
| path | TMP |
| shareType | 0 |
| shareWith | user0 |
| shareWith | user1 |
| permissions | 17 |
When As an "user0"
When As an "user1"
And creating a share with
| path | TMP |
| shareType | 3 |
@ -355,6 +357,27 @@ Feature: sharing
Then the OCS status code should be "404"
And the HTTP status code should be "200"
Scenario: do not allow to increase link share permissions on sub reshare
Given As an "admin"
And user "user0" exists
And user "user1" exists
And user "user0" created a folder "/TMP"
And user "user0" created a folder "/TMP/SUB"
And As an "user0"
And creating a share with
| path | TMP |
| shareType | 0 |
| shareWith | user1 |
| permissions | 17 |
When As an "user1"
And creating a share with
| path | TMP/SUB |
| shareType | 3 |
And Updating last share with
| publicUpload | true |
Then the OCS status code should be "404"
And the HTTP status code should be "200"
Scenario: deleting file out of a share as recipient creates a backup for the owner
Given As an "admin"
And user "user0" exists