wbrawner/server
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Make sure that permissions stay in sync for share_type 2

Browse source 
When a file/folder is shared with a group and one of the group members
moves this file/folder an extra entry is created in the share table.

When the permission of the group share is updated we used to only
sometimes update the shares for individual users.

* Added intergration tests
This commit is contained in:
Roeland Jago Douma 2015-10-09 11:57:10 +02:00
parent d7d0cfc775
commit 066e3770bb
2 changed files with 213 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

135
apps/files_sharing/tests/sharedmount.php
View file

@ -237,6 +237,141 @@ class Test_Files_Sharing_Mount extends OCA\Files_sharing\Tests\TestCase {
);
}
function dataPermissionMovedGroupShare() {
$data = [];
$powerset = function($permissions) {
$results = [\OCP\Constants::PERMISSION_READ];
foreach ($permissions as $permission) {
foreach ($results as $combination) {
$results[] = $permission | $combination;
}
}
return $results;
};
//Generate file permissions
$permissions = [
\OCP\Constants::PERMISSION_UPDATE,
\OCP\Constants::PERMISSION_CREATE,
\OCP\Constants::PERMISSION_SHARE,
];
$allPermissions = $powerset($permissions);
foreach ($allPermissions as $before) {
foreach ($allPermissions as $after) {
if ($before === $after) { continue; }
$data[] = [
'file',
$before,
$after,
];
}
}
//Generate folder permissions
$permissions = [
\OCP\Constants::PERMISSION_UPDATE,
\OCP\Constants::PERMISSION_CREATE,
\OCP\Constants::PERMISSION_SHARE,
\OCP\Constants::PERMISSION_DELETE
];
$allPermissions = $powerset($permissions);
foreach ($allPermissions as $before) {
foreach ($allPermissions as $after) {
if ($before === $after) { continue; }
$data[] = [
'folder',
$before,
$after,
];
}
}
return $data;
}
/**
* moved mountpoints of a group share should keep the same permission as their parent group share.
* See #15253
*
* @dataProvider dataPermissionMovedGroupShare
*/
function testPermissionMovedGroupShare($type, $beforePerm, $afterPerm) {
if ($type === 'file') {
$path = $this->filename;
} else if ($type === 'folder') {
$path = $this->folder;
}
\OC_Group::createGroup('testGroup');
\OC_Group::addToGroup(self::TEST_FILES_SHARING_API_USER1, 'testGroup');
\OC_Group::addToGroup(self::TEST_FILES_SHARING_API_USER2, 'testGroup');
\OC_Group::addToGroup(self::TEST_FILES_SHARING_API_USER3, 'testGroup');
// Share item with group
$fileinfo = $this->view->getFileInfo($path);
$this->assertTrue(
\OCP\Share::shareItem($type, $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, "testGroup", $beforePerm)
);
// Login as user 2 and verify the item exists
self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
$this->assertTrue(\OC\Files\Filesystem::file_exists($path));
$result = \OCP\Share::getItemSharedWithBySource($type, $fileinfo['fileid']);
$this->assertNotEmpty($result);
$this->assertEquals($beforePerm, $result['permissions']);
// Now move the item forcing a new entry in the share table
\OC\Files\Filesystem::rename($path, "newPath");
$this->assertTrue(\OC\Files\Filesystem::file_exists('newPath'));
$this->assertFalse(\OC\Files\Filesystem::file_exists($path));
// Login as user 1 again and change permissions
self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
$this->assertTrue(
\OCP\Share::setPermissions($type, $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, "testGroup", $afterPerm)
);
// Login as user 3 and verify that the permissions are changed
self::loginHelper(self::TEST_FILES_SHARING_API_USER3);
$result = \OCP\Share::getItemSharedWithBySource($type, $fileinfo['fileid']);
$this->assertNotEmpty($result);
$this->assertEquals($afterPerm, $result['permissions']);
$groupShareId = $result['id'];
// Login as user 2 and verify that the permissions are changed
self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
$result = \OCP\Share::getItemSharedWithBySource($type, $fileinfo['fileid']);
$this->assertNotEmpty($result);
$this->assertEquals($afterPerm, $result['permissions']);
$this->assertNotEquals($groupShareId, $result['id']);
// Also verify in the DB
$statement = "SELECT `permissions` FROM `*PREFIX*share` WHERE `id`=?";
$query = \OCP\DB::prepare($statement);
$result = $query->execute([$result['id']]);
$shares = $result->fetchAll();
$this->assertCount(1, $shares);
$this->assertEquals($afterPerm, $shares[0]['permissions']);
//cleanup
self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
\OCP\Share::unshare($type, $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_GROUP, 'testGroup');
\OC_Group::removeFromGroup(self::TEST_FILES_SHARING_API_USER1, 'testGroup');
\OC_Group::removeFromGroup(self::TEST_FILES_SHARING_API_USER2, 'testGroup');
\OC_Group::removeFromGroup(self::TEST_FILES_SHARING_API_USER3, 'testGroup');
}
}
class DummyTestClassSharedMount extends \OCA\Files_Sharing\SharedMount {

127
lib/private/share/share.php
View file

@ -1100,13 +1100,13 @@ class Share extends Constants {
*/
public static function setPermissions($itemType, $itemSource, $shareType, $shareWith, $permissions) {
$l = \OC::$server->getL10N('lib');
if ($item = self::getItems($itemType, $itemSource, $shareType, $shareWith,
if ($rootItem = self::getItems($itemType, $itemSource, $shareType, $shareWith,
\OC_User::getUser(), self::FORMAT_NONE, null, 1, false)) {
// Check if this item is a reshare and verify that the permissions
// granted don't exceed the parent shared item
if (isset($item['parent'])) {
if (isset($rootItem['parent'])) {
$query = \OC_DB::prepare('SELECT `permissions` FROM `*PREFIX*share` WHERE `id` = ?', 1);
$result = $query->execute(array($item['parent']))->fetchRow();
$result = $query->execute([$rootItem['parent']])->fetchRow();
if (~(int)$result['permissions'] & $permissions) {
$message = 'Setting permissions for %s failed,'
.' because the permissions exceed permissions granted to %s';
@ -1116,7 +1116,7 @@ class Share extends Constants {
}
}
$query = \OC_DB::prepare('UPDATE `*PREFIX*share` SET `permissions` = ? WHERE `id` = ?');
$query->execute(array($permissions, $item['id']));
$query->execute([$permissions, $rootItem['id']]);
if ($itemType === 'file' || $itemType === 'folder') {
\OC_Hook::emit('OCP\Share', 'post_update_permissions', array(
'itemType' => $itemType,
@ -1125,56 +1125,85 @@ class Share extends Constants {
'shareWith' => $shareWith,
'uidOwner' => \OC_User::getUser(),
'permissions' => $permissions,
'path' => $item['path'],
'share' => $item
'path' => $rootItem['path'],
'share' => $rootItem
));
}
// Check if permissions were removed
if ($item['permissions'] & ~$permissions) {
// If share permission is removed all reshares must be deleted
if (($item['permissions'] & \OCP\Constants::PERMISSION_SHARE) && (~$permissions & \OCP\Constants::PERMISSION_SHARE)) {
// delete all shares, keep parent and group children
Helper::delete($item['id'], true, null, null, true);
} else {
$ids = array();
$items = [];
$parents = array($item['id']);
while (!empty($parents)) {
$parents = "'".implode("','", $parents)."'";
$query = \OC_DB::prepare('SELECT `id`, `permissions`, `item_type` FROM `*PREFIX*share`'
.' WHERE `parent` IN ('.$parents.')');
$result = $query->execute();
// Reset parents array, only go through loop again if
// items are found that need permissions removed
$parents = array();
while ($item = $result->fetchRow()) {
$items[] = $item;
// Check if permissions need to be removed
if ($item['permissions'] & ~$permissions) {
// Add to list of items that need permissions removed
$ids[] = $item['id'];
$parents[] = $item['id'];
}
}
}
// Remove the permissions for all reshares of this item
if (!empty($ids)) {
$ids = "'".implode("','", $ids)."'";
// TODO this should be done with Doctrine platform objects
if (\OC::$server->getConfig()->getSystemValue("dbtype") === 'oci') {
$andOp = 'BITAND(`permissions`, ?)';
} else {
$andOp = '`permissions` & ?';
}
$query = \OC_DB::prepare('UPDATE `*PREFIX*share` SET `permissions` = '.$andOp
.' WHERE `id` IN ('.$ids.')');
$query->execute(array($permissions));
}
foreach ($items as $item) {
\OC_Hook::emit('OCP\Share', 'post_update_permissions', ['share' => $item]);
// Share id's to update with the new permissions
$ids = [];
$items = [];
// Check if permissions were removed
if ((int)$rootItem['permissions'] & ~$permissions) {
// If share permission is removed all reshares must be deleted
if (($rootItem['permissions'] & \OCP\Constants::PERMISSION_SHARE) && (~$permissions & \OCP\Constants::PERMISSION_SHARE)) {
// delete all shares, keep parent and group children
Helper::delete($rootItem['id'], true, null, null, true);
}
// Remove permission from all children
$parents = [$rootItem['id']];
while (!empty($parents)) {
$parents = "'".implode("','", $parents)."'";
$query = \OC_DB::prepare('SELECT `id`, `permissions`, `item_type` FROM `*PREFIX*share`'
.' WHERE `parent` IN ('.$parents.')');
$result = $query->execute();
// Reset parents array, only go through loop again if
// items are found that need permissions removed
$parents = array();
while ($item = $result->fetchRow()) {
$items[] = $item;
// Check if permissions need to be removed
if ($item['permissions'] & ~$permissions) {
// Add to list of items that need permissions removed
$ids[] = $item['id'];
$parents[] = $item['id'];
}
}
}
// Remove the permissions for all reshares of this item
if (!empty($ids)) {
$ids = "'".implode("','", $ids)."'";
// TODO this should be done with Doctrine platform objects
if (\OC::$server->getConfig()->getSystemValue("dbtype") === 'oci') {
$andOp = 'BITAND(`permissions`, ?)';
} else {
$andOp = '`permissions` & ?';
}
$query = \OC_DB::prepare('UPDATE `*PREFIX*share` SET `permissions` = '.$andOp
.' WHERE `id` IN ('.$ids.')');
$query->execute(array($permissions));
}
}
/*
* Permissions were added
* Update all USERGROUP shares. (So group shares where the user moved his mountpoint).
*/
if ($permissions & ~(int)$rootItem['permissions']) {
$query = \OC_DB::prepare('SELECT `id`, `permissions`, `item_type` FROM `*PREFIX*share`'
.' WHERE `parent` = ? AND `share_type` = ?');
$result = $query->execute([$rootItem['id'], 2]);
$ids = [];
while ($item = $result->fetchRow()) {
$items[] = $item;
$ids[] = $item['id'];
}
// Add permssions for all USERGROUP shares of this item
if (!empty($ids)) {
$ids = "'".implode("','", $ids)."'";
$query = \OC_DB::prepare('UPDATE `*PREFIX*share` SET `permissions` = ? WHERE `id` IN ('.$ids.')');
$query->execute(array($permissions));
}
}
foreach ($items as $item) {
\OC_Hook::emit('OCP\Share', 'post_update_permissions', ['share' => $item]);
}
return true;