From 4ab02c0c76e8eedeac9e58d9f196920a931e923c Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 4 Apr 2016 17:01:51 +0200 Subject: [PATCH 1/6] bring back CRUDS permissions for federated shares --- core/js/sharedialogshareelistview.js | 18 +++++------------- core/js/shareitemmodel.js | 6 ------ 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index e4edbf24c0..33f30d08fe 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -22,28 +22,25 @@ '
' + '{{/if}}' + '{{shareWithDisplayName}}' + - '{{#if mailNotificationEnabled}} {{#unless isRemoteShare}}' + + '{{#if mailNotificationEnabled}} {{#unless isRemoteShare}}' + '' + '' + '' + '' + '{{/unless}} {{/if}}' + - '{{#if isResharingAllowed}} {{#if sharePermissionPossible}} {{#unless isRemoteShare}}' + + '{{#if isResharingAllowed}} {{#if sharePermissionPossible}}' + '' + '' + '' + '' + - '{{/unless}} {{/if}} {{/if}}' + + '{{/if}} {{/if}}' + '{{#if editPermissionPossible}}' + '' + '' + '' + - '{{#unless isRemoteShare}}' + '{{crudsLabel}}' + - '{{/unless}}' + '' + '{{/if}}' + - '{{#unless isRemoteShare}}' + '' + - '{{/unless}}' + '' + '{{/each}}' + '' @@ -125,10 +121,6 @@ shareWithDisplayName = shareWithDisplayName + " (" + t('core', 'group') + ')'; } else if (shareType === OC.Share.SHARE_TYPE_REMOTE) { shareWithDisplayName = shareWithDisplayName + " (" + t('core', 'remote') + ')'; - hasPermissionOverride = { - createPermissionPossible: true, - updatePermissionPossible: true - }; } return _.extend(hasPermissionOverride, { diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js index 292230d26d..2d4af36511 100644 --- a/core/js/shareitemmodel.js +++ b/core/js/shareitemmodel.js @@ -411,12 +411,6 @@ if(!_.isObject(share)) { throw "Unknown Share"; } - if( share.share_type === OC.Share.SHARE_TYPE_REMOTE - && ( permission === OC.PERMISSION_SHARE - || permission === OC.PERMISSION_DELETE)) - { - return false; - } return (share.permissions & permission) === permission; }, From d1d80b4f9e3ddfc0e2dd509d8cb96a77251ce1db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Tue, 12 Apr 2016 12:15:00 +0200 Subject: [PATCH 2/6] set correct permissions in shareinfo --- apps/files_sharing/ajax/shareinfo.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/apps/files_sharing/ajax/shareinfo.php b/apps/files_sharing/ajax/shareinfo.php index e15e12fd28..47bc061c13 100644 --- a/apps/files_sharing/ajax/shareinfo.php +++ b/apps/files_sharing/ajax/shareinfo.php @@ -62,20 +62,25 @@ if (!$isWritable) { $rootInfo = \OC\Files\Filesystem::getFileInfo($path); $rootView = new \OC\Files\View(''); +$shareManager = \OC::$server->getShareManager(); +$share = $shareManager->getShareByToken($token); +$sharePermissions= (int)$share->getPermissions(); + /** * @param \OCP\Files\FileInfo $dir * @param \OC\Files\View $view * @return array */ -function getChildInfo($dir, $view) { +function getChildInfo($dir, $view, $sharePermissions) { $children = $view->getDirectoryContent($dir->getPath()); $result = array(); foreach ($children as $child) { $formatted = \OCA\Files\Helper::formatFileInfo($child); if ($child->getType() === 'dir') { - $formatted['children'] = getChildInfo($child, $view); + $formatted['children'] = getChildInfo($child, $view, $sharePermissions); } $formatted['mtime'] = $formatted['mtime'] / 1000; + $formatted['permissions'] = $sharePermissions & (int)$formatted['permissions']; $result[] = $formatted; } return $result; @@ -83,8 +88,11 @@ function getChildInfo($dir, $view) { $result = \OCA\Files\Helper::formatFileInfo($rootInfo); $result['mtime'] = $result['mtime'] / 1000; +$result['permissions'] = (int)$result['permissions'] & $sharePermissions; + + if ($rootInfo->getType() === 'dir') { - $result['children'] = getChildInfo($rootInfo, $rootView); + $result['children'] = getChildInfo($rootInfo, $rootView, $sharePermissions); } OCP\JSON::success(array('data' => $result)); From 52669d0ea3d687b79a9ee04c6351e6793803ecb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Wed, 13 Apr 2016 17:31:01 +0200 Subject: [PATCH 3/6] return correct share permissions on propfind --- apps/dav/lib/connector/sabre/filesplugin.php | 8 +- apps/dav/lib/connector/sabre/node.php | 29 ++++++- apps/dav/tests/unit/connector/sabre/node.php | 91 ++++++++++++-------- 3 files changed, 88 insertions(+), 40 deletions(-) diff --git a/apps/dav/lib/connector/sabre/filesplugin.php b/apps/dav/lib/connector/sabre/filesplugin.php index 686d0863f9..bd00a7c993 100644 --- a/apps/dav/lib/connector/sabre/filesplugin.php +++ b/apps/dav/lib/connector/sabre/filesplugin.php @@ -236,6 +236,8 @@ class FilesPlugin extends ServerPlugin { */ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node) { + $httpRequest = $this->server->httpRequest; + if ($node instanceof \OCA\DAV\Connector\Sabre\Node) { $propFind->handle(self::FILEID_PROPERTYNAME, function() use ($node) { @@ -255,8 +257,10 @@ class FilesPlugin extends ServerPlugin { return $perms; }); - $propFind->handle(self::SHARE_PERMISSIONS_PROPERTYNAME, function() use ($node) { - return $node->getSharePermissions(); + $propFind->handle(self::SHARE_PERMISSIONS_PROPERTYNAME, function() use ($node, $httpRequest) { + return $node->getSharePermissions( + $httpRequest->getRawServerValue('PHP_AUTH_USER') + ); }); $propFind->handle(self::GETETAG_PROPERTYNAME, function() use ($node) { diff --git a/apps/dav/lib/connector/sabre/node.php b/apps/dav/lib/connector/sabre/node.php index eaba671399..edb0ad82f4 100644 --- a/apps/dav/lib/connector/sabre/node.php +++ b/apps/dav/lib/connector/sabre/node.php @@ -32,6 +32,8 @@ namespace OCA\DAV\Connector\Sabre; use OC\Files\Mount\MoveableMount; use OCA\DAV\Connector\Sabre\Exception\InvalidPath; +use OCP\Share\Exceptions\ShareNotFound; +use OCP\Share\IManager; abstract class Node implements \Sabre\DAV\INode { @@ -60,16 +62,27 @@ abstract class Node implements \Sabre\DAV\INode { */ protected $info; + /** + * @var IManager + */ + protected $shareManager; + /** * Sets up the node, expects a full path name * * @param \OC\Files\View $view * @param \OCP\Files\FileInfo $info + * @param IManager $shareManager */ - public function __construct($view, $info) { + public function __construct($view, $info, IManager $shareManager = null) { $this->fileView = $view; $this->path = $this->fileView->getRelativePath($info->getPath()); $this->info = $info; + if ($shareManager) { + $this->shareManager = $shareManager; + } else { + $this->shareManager = \OC::$server->getShareManager(); + } } protected function refreshInfo() { @@ -215,9 +228,21 @@ abstract class Node implements \Sabre\DAV\INode { } /** + * @param string $user * @return int */ - public function getSharePermissions() { + public function getSharePermissions($user) { + + // check of we access a federated share + if ($user !== null) { + try { + $share = $this->shareManager->getShareByToken($user); + return $share->getPermissions(); + } catch (ShareNotFound $e) { + // ignore + } + } + $storage = $this->info->getStorage(); $path = $this->info->getInternalPath(); diff --git a/apps/dav/tests/unit/connector/sabre/node.php b/apps/dav/tests/unit/connector/sabre/node.php index 6fdf77adc2..f70d1ee3fa 100644 --- a/apps/dav/tests/unit/connector/sabre/node.php +++ b/apps/dav/tests/unit/connector/sabre/node.php @@ -23,6 +23,12 @@ namespace OCA\DAV\Tests\Unit\Connector\Sabre; +/** + * Class Node + * + * @group DB + * @package OCA\DAV\Tests\Unit\Connector\Sabre + */ class Node extends \Test\TestCase { public function davPermissionsProvider() { return array( @@ -66,52 +72,64 @@ class Node extends \Test\TestCase { public function sharePermissionsProvider() { return [ - [\OCP\Files\FileInfo::TYPE_FILE, 1, 1], - [\OCP\Files\FileInfo::TYPE_FILE, 3, 3], - [\OCP\Files\FileInfo::TYPE_FILE, 5, 1], - [\OCP\Files\FileInfo::TYPE_FILE, 7, 3], - [\OCP\Files\FileInfo::TYPE_FILE, 9, 1], - [\OCP\Files\FileInfo::TYPE_FILE, 11, 3], - [\OCP\Files\FileInfo::TYPE_FILE, 13, 1], - [\OCP\Files\FileInfo::TYPE_FILE, 15, 3], - [\OCP\Files\FileInfo::TYPE_FILE, 17, 17], - [\OCP\Files\FileInfo::TYPE_FILE, 19, 19], - [\OCP\Files\FileInfo::TYPE_FILE, 21, 17], - [\OCP\Files\FileInfo::TYPE_FILE, 23, 19], - [\OCP\Files\FileInfo::TYPE_FILE, 25, 17], - [\OCP\Files\FileInfo::TYPE_FILE, 27, 19], - [\OCP\Files\FileInfo::TYPE_FILE, 29, 17], - [\OCP\Files\FileInfo::TYPE_FILE, 30, 18], - [\OCP\Files\FileInfo::TYPE_FILE, 31, 19], - [\OCP\Files\FileInfo::TYPE_FOLDER, 1, 1], - [\OCP\Files\FileInfo::TYPE_FOLDER, 3, 3], - [\OCP\Files\FileInfo::TYPE_FOLDER, 5, 5], - [\OCP\Files\FileInfo::TYPE_FOLDER, 7, 7], - [\OCP\Files\FileInfo::TYPE_FOLDER, 9, 9], - [\OCP\Files\FileInfo::TYPE_FOLDER, 11, 11], - [\OCP\Files\FileInfo::TYPE_FOLDER, 13, 13], - [\OCP\Files\FileInfo::TYPE_FOLDER, 15, 15], - [\OCP\Files\FileInfo::TYPE_FOLDER, 17, 17], - [\OCP\Files\FileInfo::TYPE_FOLDER, 19, 19], - [\OCP\Files\FileInfo::TYPE_FOLDER, 21, 21], - [\OCP\Files\FileInfo::TYPE_FOLDER, 23, 23], - [\OCP\Files\FileInfo::TYPE_FOLDER, 25, 25], - [\OCP\Files\FileInfo::TYPE_FOLDER, 27, 27], - [\OCP\Files\FileInfo::TYPE_FOLDER, 29, 29], - [\OCP\Files\FileInfo::TYPE_FOLDER, 30, 30], - [\OCP\Files\FileInfo::TYPE_FOLDER, 31, 31], + [\OCP\Files\FileInfo::TYPE_FILE, null, 1, 1], + [\OCP\Files\FileInfo::TYPE_FILE, null, 3, 3], + [\OCP\Files\FileInfo::TYPE_FILE, null, 5, 1], + [\OCP\Files\FileInfo::TYPE_FILE, null, 7, 3], + [\OCP\Files\FileInfo::TYPE_FILE, null, 9, 1], + [\OCP\Files\FileInfo::TYPE_FILE, null, 11, 3], + [\OCP\Files\FileInfo::TYPE_FILE, null, 13, 1], + [\OCP\Files\FileInfo::TYPE_FILE, null, 15, 3], + [\OCP\Files\FileInfo::TYPE_FILE, null, 17, 17], + [\OCP\Files\FileInfo::TYPE_FILE, null, 19, 19], + [\OCP\Files\FileInfo::TYPE_FILE, null, 21, 17], + [\OCP\Files\FileInfo::TYPE_FILE, null, 23, 19], + [\OCP\Files\FileInfo::TYPE_FILE, null, 25, 17], + [\OCP\Files\FileInfo::TYPE_FILE, null, 27, 19], + [\OCP\Files\FileInfo::TYPE_FILE, null, 29, 17], + [\OCP\Files\FileInfo::TYPE_FILE, null, 30, 18], + [\OCP\Files\FileInfo::TYPE_FILE, null, 31, 19], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 1, 1], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 3, 3], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 5, 5], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 7, 7], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 9, 9], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 11, 11], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 13, 13], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 15, 15], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 17, 17], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 19, 19], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 21, 21], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 23, 23], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 25, 25], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 27, 27], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 29, 29], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 30, 30], + [\OCP\Files\FileInfo::TYPE_FOLDER, null, 31, 31], + [\OCP\Files\FileInfo::TYPE_FOLDER, 'shareToken', 7, 7], ]; } /** * @dataProvider sharePermissionsProvider */ - public function testSharePermissions($type, $permissions, $expected) { + public function testSharePermissions($type, $user, $permissions, $expected) { $storage = $this->getMock('\OCP\Files\Storage'); $storage->method('getPermissions')->willReturn($permissions); $mountpoint = $this->getMock('\OCP\Files\Mount\IMountPoint'); $mountpoint->method('getMountPoint')->willReturn('myPath'); + $shareManager = $this->getMockBuilder('OCP\Share\IManager')->disableOriginalConstructor()->getMock(); + $share = $this->getMockBuilder('OCP\Share\IShare')->disableOriginalConstructor()->getMock(); + + if ($user === null) { + $shareManager->expects($this->never())->method('getShareByToken'); + $share->expects($this->never())->method('getPermissions'); + } else { + $shareManager->expects($this->once())->method('getShareByToken')->with($user) + ->willReturn($share); + $share->expects($this->once())->method('getPermissions')->willReturn($permissions); + } $info = $this->getMockBuilder('\OC\Files\FileInfo') ->disableOriginalConstructor() @@ -125,6 +143,7 @@ class Node extends \Test\TestCase { $view = $this->getMock('\OC\Files\View'); $node = new \OCA\DAV\Connector\Sabre\File($view, $info); - $this->assertEquals($expected, $node->getSharePermissions()); + $this->invokePrivate($node, 'shareManager', [$shareManager]); + $this->assertEquals($expected, $node->getSharePermissions($user)); } } From e906796f99eaaea26cfbb3055ccc27024470b410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Wed, 13 Apr 2016 17:31:47 +0200 Subject: [PATCH 4/6] ask for share-permissions on propfind --- apps/files_sharing/lib/external/storage.php | 16 ++++++++++++++++ lib/private/files/storage/dav.php | 3 ++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/external/storage.php b/apps/files_sharing/lib/external/storage.php index 8fe7af6604..4382dcab0c 100644 --- a/apps/files_sharing/lib/external/storage.php +++ b/apps/files_sharing/lib/external/storage.php @@ -319,5 +319,21 @@ class Storage extends DAV implements ISharedStorage { } return ($this->getPermissions($path) & \OCP\Constants::PERMISSION_SHARE); } + + public function getPermissions($path) { + $response = $this->propfind($path); + if (isset($response['{http://open-collaboration-services.org/ns}share-permissions'])) { + $permissions = $response['{http://open-collaboration-services.org/ns}share-permissions']; + } else { + // use default permission if remote server doesn't provide the share permissions + if ($this->is_dir($path)) { + $permissions = \OCP\Constants::PERMISSION_ALL; + } else { + $permissions = \OCP\Constants::PERMISSION_ALL & ~\OCP\Constants::PERMISSION_CREATE; + } + } + + return $permissions; + } } diff --git a/lib/private/files/storage/dav.php b/lib/private/files/storage/dav.php index df0f4c7e91..a555537225 100644 --- a/lib/private/files/storage/dav.php +++ b/lib/private/files/storage/dav.php @@ -246,7 +246,7 @@ class DAV extends Common { * * @throws NotFound */ - private function propfind($path) { + protected function propfind($path) { $path = $this->cleanPath($path); $cachedResponse = $this->statCache->get($path); if ($cachedResponse === false) { @@ -264,6 +264,7 @@ class DAV extends Common { '{DAV:}getcontentlength', '{DAV:}getcontenttype', '{http://owncloud.org/ns}permissions', + '{http://open-collaboration-services.org/ns}share-permissions', '{DAV:}resourcetype', '{DAV:}getetag', ) From 2edbb7d4bd65db3aae4c2243da58c987087d32a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Fri, 15 Apr 2016 10:52:53 +0200 Subject: [PATCH 5/6] if share-permissions exists we use this information as indicator to check if the file has changed or not --- lib/private/files/storage/dav.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/private/files/storage/dav.php b/lib/private/files/storage/dav.php index a555537225..8eebea1f3b 100644 --- a/lib/private/files/storage/dav.php +++ b/lib/private/files/storage/dav.php @@ -742,6 +742,9 @@ class DAV extends Common { } if (!empty($etag) && $cachedData['etag'] !== $etag) { return true; + } else if (isset($response['{http://open-collaboration-services.org/ns}share-permissions'])) { + $sharePermissions = (int)$response['{http://open-collaboration-services.org/ns}share-permissions']; + return $sharePermissions !== $cachedData['permissions']; } else if (isset($response['{http://owncloud.org/ns}permissions'])) { $permissions = $this->parsePermissions($response['{http://owncloud.org/ns}permissions']); return $permissions !== $cachedData['permissions']; From 2a6a336e873db394e9912de20478645f3e4b8fc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Mon, 18 Apr 2016 17:15:20 +0200 Subject: [PATCH 6/6] always share with same default permissions, no special handling for remote shares --- core/js/shareitemmodel.js | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js index 2d4af36511..3ced66a1a7 100644 --- a/core/js/shareitemmodel.js +++ b/core/js/shareitemmodel.js @@ -154,21 +154,17 @@ // Default permissions are Edit (CRUD) and Share // Check if these permissions are possible var permissions = OC.PERMISSION_READ; - if (shareType === OC.Share.SHARE_TYPE_REMOTE) { - permissions = OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_READ; - } else { - if (this.updatePermissionPossible()) { - permissions = permissions | OC.PERMISSION_UPDATE; - } - if (this.createPermissionPossible()) { - permissions = permissions | OC.PERMISSION_CREATE; - } - if (this.deletePermissionPossible()) { - permissions = permissions | OC.PERMISSION_DELETE; - } - if (this.configModel.get('isResharingAllowed') && (this.sharePermissionPossible())) { - permissions = permissions | OC.PERMISSION_SHARE; - } + if (this.updatePermissionPossible()) { + permissions = permissions | OC.PERMISSION_UPDATE; + } + if (this.createPermissionPossible()) { + permissions = permissions | OC.PERMISSION_CREATE; + } + if (this.deletePermissionPossible()) { + permissions = permissions | OC.PERMISSION_DELETE; + } + if (this.configModel.get('isResharingAllowed') && (this.sharePermissionPossible())) { + permissions = permissions | OC.PERMISSION_SHARE; } attributes.permissions = permissions;