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

don't allow to share single files with delete permissions, user should only be possible to unshare a single file but never to delete it

This commit is contained in:
Bjoern Schiessle 2014-05-27 11:05:31 +02:00 committed by Robin Appelman
parent a66c2e6a47
commit bf5e9357fc
8 changed files with 413 additions and 130 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
apps/files_sharing/appinfo/update.php
View File

@ -1,6 +1,11 @@
<?php
$installedVersion = OCP\Config::getAppValue('files_sharing', 'installed_version');
if (version_compare($installedVersion, '0.5', '<')) {
updateFilePermissions();
}
if (version_compare($installedVersion, '0.4', '<')) {
removeSharedFolder();
}
@ -11,6 +16,39 @@ if (version_compare($installedVersion, '0.3.5.6', '<')) {
}
/**
* it is no longer possible to share single files with delete permissions. User
* should only be able to unshare single files but never to delete them.
*/
function updateFilePermissions($chunkSize = 99) {
$query = OCP\DB::prepare('SELECT * FROM `*PREFIX*share` WHERE item_type = ?');
$result = $query->execute(array('file'));
$updatedRows = array();
while ($row = $result->fetchRow()) {
if ($row['permissions'] & \OCP\PERMISSION_DELETE) {
$updatedRows[$row['id']] = (int)$row['permissions'] & ~\OCP\PERMISSION_DELETE;
}
}
$chunkedPermissionList = array_chunk($updatedRows, $chunkSize, true);
foreach ($chunkedPermissionList as $subList) {
$statement = "UPDATE `*PREFIX*share` SET `permissions` = CASE `id` ";
//update share table
$ids = implode(',', array_keys($subList));
foreach ($subList as $id => $permission) {
$statement .= "WHEN " . $id . " THEN " . $permission . " ";
}
$statement .= ' END WHERE `id` IN (' . $ids . ')';
$query = OCP\DB::prepare($statement);
$query->execute();
}
}
/**
* update script for the removal of the logical "Shared" folder, we create physical "Shared" folder and
* update the users file_target so that it doesn't make any difference for the user

2
apps/files_sharing/appinfo/version
View File

@ -1 +1 @@
0.4.1
0.5

9
apps/files_sharing/js/share.js
View File

@ -18,12 +18,17 @@
var oldCreateRow = OCA.Files.FileList.prototype._createRow;
OCA.Files.FileList.prototype._createRow = function(fileData) {
var tr = oldCreateRow.apply(this, arguments);
var sharePermissions = fileData.permissions;
if (fileData.type === 'file') {
// files can't be shared with delete permissions
sharePermissions = sharePermissions & ~OC.PERMISSION_DELETE;
}
tr.attr('data-share-permissions', sharePermissions);
if (fileData.shareOwner) {
tr.attr('data-share-owner', fileData.shareOwner);
// user should always be able to rename a mount point
if (fileData.isShareMountPoint) {
tr.attr('data-permissions', fileData.permissions | OC.PERMISSION_UPDATE);
tr.attr('data-reshare-permissions', fileData.permissions);
}
}
if (fileData.recipientsDisplayName) {
@ -94,7 +99,7 @@
if ($tr.data('type') === 'dir') {
itemType = 'folder';
}
var possiblePermissions = $tr.data('reshare-permissions');
var possiblePermissions = $tr.data('share-permissions');
if (_.isUndefined(possiblePermissions)) {
possiblePermissions = $tr.data('permissions');
}

23
apps/files_sharing/lib/sharedstorage.php
View File

@ -128,7 +128,18 @@ class Shared extends \OC\Files\Storage\Common {
return false;
}
/**
* Delete the directory if DELETE permission is granted
* @param string $path
* @return boolean
*/
public function rmdir($path) {
// never delete a share mount point
if(empty($path)) {
return false;
}
if (($source = $this->getSourcePath($path)) && $this->isDeletable($path)) {
list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source);
return $storage->rmdir($internalPath);
@ -256,9 +267,17 @@ class Shared extends \OC\Files\Storage\Common {
return false;
}
/**
* Delete the file if DELETE permission is granted
* @param string $path
* @return boolean
*/
public function unlink($path) {
// Delete the file if DELETE permission is granted
$path = ($path === false) ? '' : $path;
// never delete a share mount point
if (empty($path)) {
return false;
}
if ($source = $this->getSourcePath($path)) {
if ($this->isDeletable($path)) {
list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source);

108
apps/files_sharing/tests/share.php Normal file
View File

@ -0,0 +1,108 @@
<?php
/**
* ownCloud
*
* @author Bjoern Schiessle
* @copyright 2013 Bjoern Schiessle <schiessle@owncloud.com>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
* License as published by the Free Software Foundation; either
* version 3 of the License, or any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
*
* You should have received a copy of the GNU Affero General Public
* License along with this library. If not, see <http://www.gnu.org/licenses/>.
*
*/
require_once __DIR__ . '/base.php';
use OCA\Files\Share;
/**
* Class Test_Files_Sharing
*/
class Test_Files_Sharing extends Test_Files_Sharing_Base {
const TEST_FOLDER_NAME = '/folder_share_api_test';
private static $tempStorage;
function setUp() {
parent::setUp();
$this->folder = self::TEST_FOLDER_NAME;
$this->subfolder = '/subfolder_share_api_test';
$this->subsubfolder = '/subsubfolder_share_api_test';
$this->filename = '/share-api-test.txt';
// save file with content
$this->view->file_put_contents($this->filename, $this->data);
$this->view->mkdir($this->folder);
$this->view->mkdir($this->folder . $this->subfolder);
$this->view->mkdir($this->folder . $this->subfolder . $this->subsubfolder);
$this->view->file_put_contents($this->folder.$this->filename, $this->data);
$this->view->file_put_contents($this->folder . $this->subfolder . $this->filename, $this->data);
}
function tearDown() {
$this->view->unlink($this->filename);
$this->view->deleteAll($this->folder);
self::$tempStorage = null;
parent::tearDown();
}
/**
* shared files should never have delete permissions
* @dataProvider DataProviderTestFileSharePermissions
*/
function testFileSharePermissions($permission, $expectedPermissions) {
$fileinfo = $this->view->getFileInfo($this->filename);
$result = \OCP\Share::shareItem('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER,
\Test_Files_Sharing::TEST_FILES_SHARING_API_USER2, $permission);
$this->assertTrue($result);
$result = \OCP\Share::getItemShared('file', null);
$this->assertTrue(is_array($result));
// test should return exactly one shares created from testCreateShare()
$this->assertTrue(count($result) === 1);
$share = reset($result);
$this->assertSame($expectedPermissions, $share['permissions']);
\OCP\Share::unshare('file', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER,
\Test_Files_Sharing::TEST_FILES_SHARING_API_USER2);
}
function DataProviderTestFileSharePermissions() {
$permission1 = \OCP\PERMISSION_ALL;
$permission2 = \OCP\PERMISSION_DELETE;
$permission3 = \OCP\PERMISSION_READ;
$permission4 = \OCP\PERMISSION_READ | \OCP\PERMISSION_UPDATE;
$permission5 = \OCP\PERMISSION_READ | \OCP\PERMISSION_DELETE;
$permission6 = \OCP\PERMISSION_READ | \OCP\PERMISSION_UPDATE | \OCP\PERMISSION_DELETE;
return array(
array($permission1, \OCP\PERMISSION_ALL & ~\OCP\PERMISSION_DELETE),
array($permission2, 0),
array($permission3, $permission3),
array($permission4, $permission4),
array($permission5, $permission3),
array($permission6, $permission4),
);
}
}

233
apps/files_sharing/tests/update.php Normal file
View File

@ -0,0 +1,233 @@
<?php
/**
* ownCloud
*
* @author Morris Jobke, Bjoern Schiessle
* @copyright 2014 Morris Jobke <morris.jobke@gmail.com>
* 2014 Bjoern Schiessle <schiessle@ownlcoud.com>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
* License as published by the Free Software Foundation; either
* version 3 of the License, or any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
*
* You should have received a copy of the GNU Affero General Public
* License along with this library. If not, see <http://www.gnu.org/licenses/>.
*
*/
require_once __DIR__ . '/../appinfo/update.php';
require_once __DIR__ . '/base.php';
/**
* Class Test_Files_Sharing_Update
*/
class Test_Files_Sharing_Update_Routine extends Test_Files_Sharing_Base {
const TEST_FOLDER_NAME = '/folder_share_api_test';
function setUp() {
parent::setUp();
$this->folder = self::TEST_FOLDER_NAME;
$this->filename = '/share-api-test.txt';
// save file with content
$this->view->file_put_contents($this->filename, $this->data);
$this->view->mkdir($this->folder);
$this->view->file_put_contents($this->folder . '/' . $this->filename, $this->data);
}
function tearDown() {
$this->view->unlink($this->filename);
$this->view->deleteAll($this->folder);
$removeShares = \OC_DB::prepare('DELETE FROM `*PREFIX*share`');
$removeShares->execute();
$removeItems = \OC_DB::prepare('DELETE FROM `*PREFIX*filecache`');
$removeItems->execute();
parent::tearDown();
}
/**
* test update of file permission. The update should remove from all shared
* files the delete permission
*/
function testUpdateFilePermissions() {
self::prepareDBUpdateFilePermissions();
// run the update routine to update the share permission
updateFilePermissions(2);
// verify results
$query = \OC_DB::prepare('SELECT * FROM `*PREFIX*share`');
$result = $query->execute(array());
while ($row = $result->fetchRow()) {
if ($row['item_type'] === 'file') {
// for all files the delete permission should be removed
$this->assertSame(0, (int)$row['permissions'] & \OCP\PERMISSION_DELETE);
} else {
// for all other the permission shouldn't change
$this->assertSame(31, (int)$row['permissions'] & \OCP\PERMISSION_ALL);
}
}
// cleanup
$this->cleanupSharedTable();
}
/**
* @medium
*/
function testRemoveBrokenShares() {
$this->prepareFileCache();
// check if there are just 3 shares (see setUp - precondition: empty table)
$countShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share`');
$result = $countShares->execute()->fetchOne();
$this->assertEquals(3, $result);
// check if there are just 2 items (see setUp - precondition: empty table)
$countItems = \OC_DB::prepare('SELECT COUNT(`fileid`) FROM `*PREFIX*filecache`');
$result = $countItems->execute()->fetchOne();
$this->assertEquals(2, $result);
// execute actual code which should be tested
\OC\Files\Cache\Shared_Updater::fixBrokenSharesOnAppUpdate();
// check if there are just 2 shares (one gets killed by the code as there is no filecache entry for this)
$countShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share`');
$result = $countShares->execute()->fetchOne();
$this->assertEquals(2, $result);
// check if the share of file '200' is removed as there is no entry for this in filecache table
$countShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share` WHERE `file_source` = 200');
$result = $countShares->execute()->fetchOne();
$this->assertEquals(0, $result);
// check if there are just 2 items
$countItems = \OC_DB::prepare('SELECT COUNT(`fileid`) FROM `*PREFIX*filecache`');
$result = $countItems->execute()->fetchOne();
$this->assertEquals(2, $result);
}
/**
* test update for the removal of the logical "Shared" folder. It should update
* the file_target for every share and create a physical "Shared" folder for each user
*/
function testRemoveSharedFolder() {
self::prepareDB();
// run the update routine to remove the shared folder and replace it with a real folder
removeSharedFolder(false, 2);
// verify results
$query = \OC_DB::prepare('SELECT * FROM `*PREFIX*share`');
$result = $query->execute(array());
$newDBContent = $result->fetchAll();
foreach ($newDBContent as $row) {
if ((int)$row['share_type'] === \OCP\Share::SHARE_TYPE_USER) {
$this->assertSame('/Shared', substr($row['file_target'], 0, strlen('/Shared')));
} else {
$this->assertSame('/ShouldNotChange', $row['file_target']);
}
}
// cleanup
$this->cleanupSharedTable();
}
private function cleanupSharedTable() {
$query = \OC_DB::prepare('DELETE FROM `*PREFIX*share`');
$query->execute();
}
/**
* prepare sharing table for testRemoveSharedFolder()
*/
private function prepareDB() {
$this->cleanupSharedTable();
// add items except one - because this is the test case for the broken share table
$addItems = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (`share_type`, `item_type`, ' .
'`share_with`, `uid_owner` , `file_target`) ' .
'VALUES (?, ?, ?, ?, ?)');
$items = array(
array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user1', 'admin' , '/foo'),
array(\OCP\Share::SHARE_TYPE_USER, 'folder', 'user2', 'admin', '/foo2'),
array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user3', 'admin', '/foo3'),
array(\OCP\Share::SHARE_TYPE_USER, 'folder', 'user4', 'admin', '/foo4'),
array(\OCP\Share::SHARE_TYPE_LINK, 'file', 'user1', 'admin', '/ShouldNotChange'),
array(\OCP\Share::SHARE_TYPE_CONTACT, 'contact', 'admin', 'user1', '/ShouldNotChange'),
);
foreach($items as $item) {
// the number is used as path_hash
$addItems->execute($item);
}
}
/**
* prepare sharing table for testUpdateFilePermissions()
*/
private function prepareDBUpdateFilePermissions() {
$this->cleanupSharedTable();
// add items except one - because this is the test case for the broken share table
$addItems = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (`share_type`, `item_type`, ' .
'`share_with`, `uid_owner` , `file_target`, `permissions`) ' .
'VALUES (?, ?, ?, ?, ?, ?)');
$items = array(
array(\OCP\Share::SHARE_TYPE_LINK, 'file', 'user1', 'admin', '/foo', \OCP\PERMISSION_ALL),
array(\OCP\Share::SHARE_TYPE_CONTACT, 'contact', 'admin', 'user1', '/foo', \OCP\PERMISSION_ALL),
array(\OCP\Share::SHARE_TYPE_USER, 'folder', 'user4', 'admin', '/foo', \OCP\PERMISSION_ALL),
array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user3', 'admin', '/foo3', \OCP\PERMISSION_ALL),
array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user1', 'admin' , '/foo', \OCP\PERMISSION_DELETE),
array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user1', 'admin' , '/foo', \OCP\PERMISSION_READ & \OCP\PERMISSION_DELETE),
array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user1', 'admin' , '/foo', \OCP\PERMISSION_SHARE & \OCP\PERMISSION_UPDATE),
array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user1', 'admin' , '/foo', \OCP\PERMISSION_ALL),
array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user1', 'admin' , '/foo', \OCP\PERMISSION_SHARE & \OCP\PERMISSION_READ & \OCP\PERMISSION_DELETE),
);
foreach($items as $item) {
// the number is used as path_hash
$addItems->execute($item);
}
}
/**
* prepare file cache for testRemoveBrokenShares()
*/
private function prepareFileCache() {
// some previous tests didn't clean up and therefore this has to be done here
// FIXME: DIRTY HACK - TODO: find tests, that don't clean up and fix it there
$this->tearDown();
// add items except one - because this is the test case for the broken share table
$addItems = \OC_DB::prepare('INSERT INTO `*PREFIX*filecache` (`storage`, `path_hash`, ' .
'`parent`, `mimetype`, `mimepart`, `size`, `mtime`, `storage_mtime`) ' .
'VALUES (1, ?, 1, 1, 1, 1, 1, 1)');
$items = array(1, 3);
$fileIds = array();
foreach($items as $item) {
// the number is used as path_hash
$addItems->execute(array($item));
$fileIds[] = \OC_DB::insertId('*PREFIX*filecache');
}
$addShares = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (`file_source`, `item_type`, `uid_owner`) VALUES (?, \'file\', 1)');
// the number is used as item_source
$addShares->execute(array($fileIds[0]));
$addShares->execute(array(200)); // id of "deleted" file
$addShares->execute(array($fileIds[1]));
}
}

125
apps/files_sharing/tests/updater.php
View File

@ -47,11 +47,6 @@ class Test_Files_Sharing_Updater extends Test_Files_Sharing_Base {
$this->view->unlink($this->filename);
$this->view->deleteAll($this->folder);
$removeShares = \OC_DB::prepare('DELETE FROM `*PREFIX*share`');
$removeShares->execute();
$removeItems = \OC_DB::prepare('DELETE FROM `*PREFIX*filecache`');
$removeItems->execute();
parent::tearDown();
}
@ -111,124 +106,4 @@ class Test_Files_Sharing_Updater extends Test_Files_Sharing_Base {
}
}
/**
* @medium
*/
function testRemoveBrokenShares() {
$this->prepareFileCache();
// check if there are just 3 shares (see setUp - precondition: empty table)
$countShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share`');
$result = $countShares->execute()->fetchOne();
$this->assertEquals(3, $result);
// check if there are just 2 items (see setUp - precondition: empty table)
$countItems = \OC_DB::prepare('SELECT COUNT(`fileid`) FROM `*PREFIX*filecache`');
$result = $countItems->execute()->fetchOne();
$this->assertEquals(2, $result);
// execute actual code which should be tested
\OC\Files\Cache\Shared_Updater::fixBrokenSharesOnAppUpdate();
// check if there are just 2 shares (one gets killed by the code as there is no filecache entry for this)
$countShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share`');
$result = $countShares->execute()->fetchOne();
$this->assertEquals(2, $result);
// check if the share of file '200' is removed as there is no entry for this in filecache table
$countShares = \OC_DB::prepare('SELECT COUNT(`id`) FROM `*PREFIX*share` WHERE `file_source` = 200');
$result = $countShares->execute()->fetchOne();
$this->assertEquals(0, $result);
// check if there are just 2 items
$countItems = \OC_DB::prepare('SELECT COUNT(`fileid`) FROM `*PREFIX*filecache`');
$result = $countItems->execute()->fetchOne();
$this->assertEquals(2, $result);
}
/**
* test update for the removal of the logical "Shared" folder. It should update
* the file_target for every share and create a physical "Shared" folder for each user
*/
function testRemoveSharedFolder() {
self::prepareDB();
// run the update routine to remove the shared folder and replace it with a real folder
removeSharedFolder(false, 2);
// verify results
$query = \OC_DB::prepare('SELECT * FROM `*PREFIX*share`');
$result = $query->execute(array());
$newDBContent = $result->fetchAll();
foreach ($newDBContent as $row) {
if ((int)$row['share_type'] === \OCP\Share::SHARE_TYPE_USER) {
$this->assertSame('/Shared', substr($row['file_target'], 0, strlen('/Shared')));
} else {
$this->assertSame('/ShouldNotChange', $row['file_target']);
}
}
// cleanup
$this->cleanupSharedTable();
}
private function cleanupSharedTable() {
$query = \OC_DB::prepare('DELETE FROM `*PREFIX*share`');
$query->execute();
}
/**
* prepare sharing table for testRemoveSharedFolder()
*/
private function prepareDB() {
$this->cleanupSharedTable();
// add items except one - because this is the test case for the broken share table
$addItems = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (`share_type`, `item_type`, ' .
'`share_with`, `uid_owner` , `file_target`) ' .
'VALUES (?, ?, ?, ?, ?)');
$items = array(
array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user1', 'admin' , '/foo'),
array(\OCP\Share::SHARE_TYPE_USER, 'folder', 'user2', 'admin', '/foo2'),
array(\OCP\Share::SHARE_TYPE_USER, 'file', 'user3', 'admin', '/foo3'),
array(\OCP\Share::SHARE_TYPE_USER, 'folder', 'user4', 'admin', '/foo4'),
array(\OCP\Share::SHARE_TYPE_LINK, 'file', 'user1', 'admin', '/ShouldNotChange'),
array(\OCP\Share::SHARE_TYPE_CONTACT, 'contact', 'admin', 'user1', '/ShouldNotChange'),
);
foreach($items as $item) {
// the number is used as path_hash
$addItems->execute($item);
}
}
/**
* prepare file cache for testRemoveBrokenShares()
*/
private function prepareFileCache() {
// some previous tests didn't clean up and therefore this has to be done here
// FIXME: DIRTY HACK - TODO: find tests, that don't clean up and fix it there
$this->tearDown();
// add items except one - because this is the test case for the broken share table
$addItems = \OC_DB::prepare('INSERT INTO `*PREFIX*filecache` (`storage`, `path_hash`, ' .
'`parent`, `mimetype`, `mimepart`, `size`, `mtime`, `storage_mtime`) ' .
'VALUES (1, ?, 1, 1, 1, 1, 1, 1)');
$items = array(1, 3);
$fileIds = array();
foreach($items as $item) {
// the number is used as path_hash
$addItems->execute(array($item));
$fileIds[] = \OC_DB::insertId('*PREFIX*filecache');
}
$addShares = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (`file_source`, `item_type`, `uid_owner`) VALUES (?, \'file\', 1)');
// the number is used as item_source
$addShares->execute(array($fileIds[0]));
$addShares->execute(array(200)); // id of "deleted" file
$addShares->execute(array($fileIds[1]));
}
}

5
lib/private/share/share.php
View File

@ -519,6 +519,11 @@ class Share extends \OC\Share\Constants {
}
}
// single file shares should never have delete permissions
if ($itemType === 'file') {
$permissions = (int)$permissions & ~\OCP\PERMISSION_DELETE;
}
// Verify share type and sharing conditions are met
if ($shareType === self::SHARE_TYPE_USER) {
if ($shareWith == $uidOwner) {