Better check reshare permissions when creating a share

Signed-off-by: Joas Schilling <coding@schilljs.com>
This commit is contained in:
Joas Schilling 2019-07-02 10:22:30 +02:00
parent eaabe97373
commit e4addbae3e
No known key found for this signature in database
GPG Key ID: 7076EA9751AACDDA
3 changed files with 88 additions and 3 deletions

View File

@ -251,6 +251,66 @@ 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 additional delete permissions
As an "admin"
Given user "user0" exists
And user "user1" exists
And user "user2" exists
And As an "user0"
And creating a share with
| path | /PARENT |
| shareType | 0 |
| shareWith | user1 |
| permissions | 16 |
And As an "user1"
When creating a share with
| path | /PARENT (2) |
| shareType | 0 |
| shareWith | user2 |
| permissions | 25 |
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 for files
As an "admin"
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 | 16 |
And As an "user1"
When creating a share with
| path | /textfile0 (2).txt |
| shareType | 0 |
| shareWith | user2 |
| permissions | 25 |
Then the OCS status code should be "100"
And the HTTP status code should be "200"
When Getting info of last share
Then Share fields of last share match with
| id | A_NUMBER |
| item_type | file |
| item_source | A_NUMBER |
| share_type | 0 |
| share_with | user2 |
| file_source | A_NUMBER |
| file_target | /textfile0 (2).txt |
| path | /textfile0 (2).txt |
| permissions | 17 |
| stime | A_NUMBER |
| storage | A_NUMBER |
| mail_send | 0 |
| uid_owner | user1 |
| storage_id | shared::/textfile0 (2).txt |
| file_parent | A_NUMBER |
| share_with_displayname | user2 |
| displayname_owner | user1 |
| mimetype | text/plain |
Scenario: Get a share with a user which didn't received the share
Given user "user0" exists
And user "user1" exists

View File

@ -269,11 +269,13 @@ class Manager implements IManager {
// And you can't share your rootfolder
if ($this->userManager->userExists($share->getSharedBy())) {
$sharedPath = $this->rootFolder->getUserFolder($share->getSharedBy())->getPath();
$userFolder = $this->rootFolder->getUserFolder($share->getSharedBy());
$userFolderPath = $userFolder->getPath();
} else {
$sharedPath = $this->rootFolder->getUserFolder($share->getShareOwner())->getPath();
$userFolder = $this->rootFolder->getUserFolder($share->getShareOwner());
$userFolderPath = $userFolder->getPath();
}
if ($sharedPath === $share->getNode()->getPath()) {
if ($userFolderPath === $share->getNode()->getPath()) {
throw new \InvalidArgumentException('You cant share your root folder');
}
@ -297,6 +299,23 @@ class Manager implements IManager {
$mount = $share->getNode()->getMountPoint();
if (!($mount instanceof MoveableMount)) {
$permissions |= \OCP\Constants::PERMISSION_DELETE | \OCP\Constants::PERMISSION_UPDATE;
} else if ($share->getNode()->getOwner()->getUID() !== $share->getSharedBy()) {
$userMountPointId = $mount->getStorageRootId();
$userMountPoints = $userFolder->getById($userMountPointId);
$userMountPoint = array_shift($userMountPoints);
/* Check if this is an incoming share */
$incomingShares = $this->getSharedWith($share->getSharedBy(), Share::SHARE_TYPE_USER, $userMountPoint, -1, 0);
$incomingShares = array_merge($incomingShares, $this->getSharedWith($share->getSharedBy(), Share::SHARE_TYPE_GROUP, $userMountPoint, -1, 0));
$incomingShares = array_merge($incomingShares, $this->getSharedWith($share->getSharedBy(), Share::SHARE_TYPE_ROOM, $userMountPoint, -1, 0));
/** @var \OCP\Share\IShare[] $incomingShares */
if (!empty($incomingShares)) {
$permissions = 0;
foreach ($incomingShares as $incomingShare) {
$permissions |= $incomingShare->getPermissions();
}
}
}
// Check that we do not share with more permissions than we have

View File

@ -589,6 +589,12 @@ class ManagerTest extends \Test\TestCase {
$limitedPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ);
$limitedPermssions->method('getPath')->willReturn('path');
$owner = $this->createMock(IUser::class);
$owner->method('getUID')
->willReturn($user0);
$limitedPermssions->method('getOwner')
->willReturn($owner);
$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];
$data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_LINK, $limitedPermssions, null, $user0, $user0, null, null, null), 'A share requires permissions', true];