From 0805ef5c8bcac3ce50c0b9f635c690b6432b39ce Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 8 Aug 2017 12:57:07 +0200 Subject: [PATCH 1/8] Expose capabilities in core JS Capabilities are computed in js/config.php and stored in oc_capabilities and accessible under OC.getCapabilities() on the JS side. This is only exposed if the user has logged in. --- core/js/config.php | 4 ++++ core/js/js.js | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/core/js/config.php b/core/js/config.php index bc82b859b171..1f124d5c93a4 100644 --- a/core/js/config.php +++ b/core/js/config.php @@ -198,6 +198,10 @@ $array['oc_config']['versionstring'] = OC_Util::getVersionString(); $array['oc_defaults']['docBaseUrl'] = $defaults->getDocBaseUrl(); $array['oc_defaults']['docPlaceholderUrl'] = $defaults->buildDocLinkToKey('PLACEHOLDER'); + $caps = \OC::$server->getCapabilitiesManager()->getCapabilities(); + // remove status.php info as we already have the version above + unset($caps['core']['status']); + $array['oc_capabilities'] = json_encode($caps); } // Allow hooks to modify the output values diff --git a/core/js/js.js b/core/js/js.js index ad82e4b30d1c..214dfa998c26 100644 --- a/core/js/js.js +++ b/core/js/js.js @@ -68,6 +68,13 @@ var OC = { */ webroot: oc_webroot, + /** + * Capabilities + * + * @type array + */ + _capabilities: window.oc_capabilities || null, + appswebroots: (typeof oc_appswebroots !== 'undefined') ? oc_appswebroots : false, /** * Currently logged in user or null if none @@ -298,6 +305,15 @@ var OC = { return OC.webroot; }, + /** + * Returns the capabilities + * + * @return {array} capabilities + */ + getCapabilities: function() { + return OC._capabilities; + }, + /** * Returns the currently logged in user or null if there is no logged in * user (public page mode) From 4527cd50f8bc618d9ab95b58b00b5e6f18d0f7c5 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 8 Aug 2017 13:06:55 +0200 Subject: [PATCH 2/8] Add setting for default share permissions --- apps/files_sharing/lib/Capabilities.php | 2 ++ core/js/shareitemmodel.js | 14 ++++++++------ settings/templates/panels/admin/filesharing.php | 5 +++++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/apps/files_sharing/lib/Capabilities.php b/apps/files_sharing/lib/Capabilities.php index 40aaf29dc94b..c07fe1c7c34a 100644 --- a/apps/files_sharing/lib/Capabilities.php +++ b/apps/files_sharing/lib/Capabilities.php @@ -78,6 +78,8 @@ public function getCapabilities() { $res['resharing'] = $this->config->getAppValue('core', 'shareapi_allow_resharing', 'yes') === 'yes'; $res['group_sharing'] = $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes') === 'yes'; + + $res['default_permissions'] = (int)$this->config->getAppValue('core', 'shareapi_default_permissions', \OCP\Constants::PERMISSION_ALL); } //Federated sharing diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js index fa6c04d7cf50..aecea435c6e3 100644 --- a/core/js/shareitemmodel.js +++ b/core/js/shareitemmodel.js @@ -126,23 +126,25 @@ options = options || {}; attributes = _.extend({}, attributes); + var defaultPermissions = OC.getCapabilities()['files_sharing']['default_permissions'] || OC.PERMISSION_ALL; + // Default permissions are Edit (CRUD) and Share // Check if these permissions are possible - var permissions = OC.PERMISSION_READ; + var possiblePermissions = OC.PERMISSION_READ; if (this.updatePermissionPossible()) { - permissions = permissions | OC.PERMISSION_UPDATE; + possiblePermissions = possiblePermissions | OC.PERMISSION_UPDATE; } if (this.createPermissionPossible()) { - permissions = permissions | OC.PERMISSION_CREATE; + possiblePermissions = possiblePermissions | OC.PERMISSION_CREATE; } if (this.deletePermissionPossible()) { - permissions = permissions | OC.PERMISSION_DELETE; + possiblePermissions = possiblePermissions | OC.PERMISSION_DELETE; } if (this.configModel.get('isResharingAllowed') && (this.sharePermissionPossible())) { - permissions = permissions | OC.PERMISSION_SHARE; + possiblePermissions = possiblePermissions | OC.PERMISSION_SHARE; } - attributes.permissions = permissions; + attributes.permissions = defaultPermissions & possiblePermissions; if (_.isUndefined(attributes.path)) { attributes.path = this.fileInfoModel.getFullPath(); } diff --git a/settings/templates/panels/admin/filesharing.php b/settings/templates/panels/admin/filesharing.php index 632d4cd8b03c..2989198d5bbd 100644 --- a/settings/templates/panels/admin/filesharing.php +++ b/settings/templates/panels/admin/filesharing.php @@ -59,6 +59,11 @@ value="1" />

+

+ /> +
+

/> From 8e0bbe469d73d1da5e4810b8b67187091a4721ba Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 1 Sep 2017 11:58:36 +0200 Subject: [PATCH 3/8] Details default share permissions in settings page --- lib/private/Settings/SettingsManager.php | 2 +- settings/Panels/Admin/FileSharing.php | 31 ++++++++++++++++++- settings/js/admin.js | 24 +++++++++++++- .../templates/panels/admin/filesharing.php | 15 ++++++--- 4 files changed, 65 insertions(+), 7 deletions(-) diff --git a/lib/private/Settings/SettingsManager.php b/lib/private/Settings/SettingsManager.php index 3724eb7390a2..761af4bd33ff 100644 --- a/lib/private/Settings/SettingsManager.php +++ b/lib/private/Settings/SettingsManager.php @@ -282,7 +282,7 @@ public function getBuiltInPanel($className) { $this->urlGenerator, $this->certificateManager), Encryption::class => new Encryption(), - FileSharing::class => new FileSharing($this->config, $this->helper), + FileSharing::class => new FileSharing($this->config, $this->helper, $this->l), Logging::class => new Logging($this->config, $this->urlGenerator, $this->helper), Mail::class => new Mail($this->config, $this->helper), SecurityWarning::class => new SecurityWarning( diff --git a/settings/Panels/Admin/FileSharing.php b/settings/Panels/Admin/FileSharing.php index 88c8729a195d..aa509107e03d 100644 --- a/settings/Panels/Admin/FileSharing.php +++ b/settings/Panels/Admin/FileSharing.php @@ -25,6 +25,7 @@ use OCP\IConfig; use OCP\Settings\ISettings; use OCP\Template; +use OCP\IL10N; class FileSharing implements ISettings { @@ -32,10 +33,13 @@ class FileSharing implements ISettings { protected $config; /** @var Helper */ protected $helper; + /** @var IL10N */ + protected $l; - public function __construct(IConfig $config, Helper $helper) { + public function __construct(IConfig $config, Helper $helper, IL10N $l) { $this->config = $config; $this->helper = $helper; + $this->l = $l; } public function getPriority() { @@ -64,6 +68,31 @@ public function getPanel() { $template->assign('shareExcludedGroupsList', !is_null($excludedGroupsList) ? implode('|', $excludedGroupsList) : ''); $template->assign('shareExpireAfterNDays', $this->config->getAppValue('core', 'shareapi_expire_after_n_days', '7')); $template->assign('shareEnforceExpireDate', $this->config->getAppValue('core', 'shareapi_enforce_expire_date', 'no')); + + $permList = [ + [ + 'id' => 'cancreate', + 'label' => $this->l->t('Create'), + 'value' => \OCP\Constants::PERMISSION_CREATE + ], + [ + 'id' => 'canupdate', + 'label' => $this->l->t('Update'), + 'value' => \OCP\Constants::PERMISSION_UPDATE + ], + [ + 'id' => 'candelete', + 'label' => $this->l->t('Delete'), + 'value' => \OCP\Constants::PERMISSION_DELETE + ], + [ + 'id' => 'canshare', + 'label' => $this->l->t('Share'), + 'value' => \OCP\Constants::PERMISSION_SHARE + ], + ]; + $template->assign('shareApiDefaultPermissions', $this->config->getAppValue('core', 'shareapi_default_permissions', \OCP\Constants::PERMISSION_ALL)); + $template->assign('shareApiDefaultPermissionsCheckboxes', $permList); return $template; } diff --git a/settings/js/admin.js b/settings/js/admin.js index b02ce51aad4f..ccce5ddaf625 100644 --- a/settings/js/admin.js +++ b/settings/js/admin.js @@ -57,7 +57,7 @@ $(document).ready(function(){ } }); - $('#shareAPI input:not(#excludedGroups)').change(function() { + $('#shareAPI input:not(.noautosave)').change(function() { var value = $(this).val(); if ($(this).attr('type') === 'checkbox') { if (this.checked) { @@ -87,5 +87,27 @@ $(document).ready(function(){ $("#selectExcludedGroups").toggleClass('hidden', !this.checked); }); + $('#shareApiDefaultPermissionsSection input').change(function(ev) { + var $el = $('#shareApiDefaultPermissions'); + var $target = $(ev.target); + + var value = $el.val(); + if ($target.is(':checked')) { + value = value | $target.val(); + } else { + value = value & ~$target.val(); + } + + // always set read permission + value |= OC.PERMISSION_READ; + + // this will trigger the field's change event and will save it + $el.val(value).change(); + + ev.preventDefault(); + + return false; + }); + }); diff --git a/settings/templates/panels/admin/filesharing.php b/settings/templates/panels/admin/filesharing.php index 2989198d5bbd..7619b03ef873 100644 --- a/settings/templates/panels/admin/filesharing.php +++ b/settings/templates/panels/admin/filesharing.php @@ -60,9 +60,16 @@

- /> -
+ + t('Default local share permissions'));?> +

+

+ + /> +
+

t('Exclude groups from sharing'));?>

- +
t('These groups will still be able to receive shares, but not to initiate them.')); ?>

From a186fa1896cf242a001b31f4497b24065a51b503 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 1 Sep 2017 12:29:41 +0200 Subject: [PATCH 4/8] OCS API now uses default share perms if none passed at creation time --- .../files_sharing/lib/API/OCSShareWrapper.php | 3 ++- apps/files_sharing/lib/API/Share20OCS.php | 20 +++++++++++++++---- .../tests/API/Share20OCSTest.php | 18 +++++++++++++++-- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/apps/files_sharing/lib/API/OCSShareWrapper.php b/apps/files_sharing/lib/API/OCSShareWrapper.php index 8b6db3b41212..ab8a446f3d09 100644 --- a/apps/files_sharing/lib/API/OCSShareWrapper.php +++ b/apps/files_sharing/lib/API/OCSShareWrapper.php @@ -34,7 +34,8 @@ private function getShare20OCS() { \OC::$server->getRootFolder(), \OC::$server->getURLGenerator(), \OC::$server->getUserSession()->getUser(), - \OC::$server->getL10N('files_sharing') + \OC::$server->getL10N('files_sharing'), + \OC::$server->getConfig() ); } diff --git a/apps/files_sharing/lib/API/Share20OCS.php b/apps/files_sharing/lib/API/Share20OCS.php index 89b1d413f329..0c406325a119 100644 --- a/apps/files_sharing/lib/API/Share20OCS.php +++ b/apps/files_sharing/lib/API/Share20OCS.php @@ -36,6 +36,7 @@ use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\Exceptions\GenericShareException; use OCP\Lock\ILockingProvider; +use OCP\IConfig; /** * Class Share20OCS @@ -60,6 +61,8 @@ class Share20OCS { private $currentUser; /** @var IL10N */ private $l; + /** @var IConfig */ + private $config; /** * Share20OCS constructor. @@ -71,6 +74,8 @@ class Share20OCS { * @param IRootFolder $rootFolder * @param IURLGenerator $urlGenerator * @param IUser $currentUser + * @param IL10N $l10n + * @param IConfig $config */ public function __construct( IManager $shareManager, @@ -80,7 +85,8 @@ public function __construct( IRootFolder $rootFolder, IURLGenerator $urlGenerator, IUser $currentUser, - IL10N $l10n + IL10N $l10n, + IConfig $config ) { $this->shareManager = $shareManager; $this->userManager = $userManager; @@ -90,6 +96,7 @@ public function __construct( $this->urlGenerator = $urlGenerator; $this->currentUser = $currentUser; $this->l = $l10n; + $this->config = $config; } /** @@ -270,10 +277,17 @@ public function createShare() { return new \OC\OCS\Result(null, 404, 'Could not create share'); } + $shareType = (int)$this->request->getParam('shareType', '-1'); + // Parse permissions (if available) $permissions = $this->request->getParam('permissions', null); if ($permissions === null) { - $permissions = \OCP\Constants::PERMISSION_ALL; + if ($shareType !== \OCP\Share::SHARE_TYPE_LINK) { + $permissions = $this->config->getAppValue('core', 'shareapi_default_permissions', \OCP\Constants::PERMISSION_ALL); + $permissions |= \OCP\Constants::PERMISSION_READ; + } else { + $permissions = \OCP\Constants::PERMISSION_ALL; + } } else { $permissions = (int)$permissions; } @@ -287,8 +301,6 @@ public function createShare() { return new \OC\OCS\Result(null, 400, $this->l->t('Cannot remove all permissions')); } - $shareType = (int)$this->request->getParam('shareType', '-1'); - // link shares can have create-only without read (anonymous upload) if ($shareType !== \OCP\Share::SHARE_TYPE_LINK && $permissions !== \OCP\Constants::PERMISSION_CREATE) { // Shares always require read permissions diff --git a/apps/files_sharing/tests/API/Share20OCSTest.php b/apps/files_sharing/tests/API/Share20OCSTest.php index 022fe2db7865..4827946243bc 100644 --- a/apps/files_sharing/tests/API/Share20OCSTest.php +++ b/apps/files_sharing/tests/API/Share20OCSTest.php @@ -36,6 +36,7 @@ use OCP\Lock\LockedException; use OCP\Share; use Test\TestCase; +use OCP\IConfig; /** * Class Share20OCSTest @@ -96,6 +97,12 @@ protected function setUp() { return vsprintf($text, $parameters); })); + $this->config = $this->createMock(IConfig::class); + $this->config->method('getAppValue') + ->will($this->returnValueMap([ + ['core', 'shareapi_default_permissions', \OCP\Constants::PERMISSION_ALL, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE] + ])); + $this->ocs = new Share20OCS( $this->shareManager, $this->groupManager, @@ -104,7 +111,8 @@ protected function setUp() { $this->rootFolder, $this->urlGenerator, $this->currentUser, - $this->l + $this->l, + $this->config ); } @@ -119,6 +127,7 @@ private function mockFormatShare() { $this->urlGenerator, $this->currentUser, $this->l, + $this->config, ])->setMethods(['formatShare']) ->getMock(); } @@ -425,6 +434,7 @@ public function testGetShare(\OCP\Share\IShare $share, array $result) { $this->urlGenerator, $this->currentUser, $this->l, + $this->config, ])->setMethods(['canAccessShare']) ->getMock(); @@ -751,6 +761,7 @@ public function testCreateShareUser() { $this->urlGenerator, $this->currentUser, $this->l, + $this->config, ])->setMethods(['formatShare']) ->getMock(); @@ -867,6 +878,7 @@ public function testCreateShareGroup() { $this->urlGenerator, $this->currentUser, $this->l, + $this->config, ])->setMethods(['formatShare']) ->getMock(); @@ -1403,6 +1415,7 @@ public function testCreateReshareOfFederatedMountNoDeletePermissions() { $this->urlGenerator, $this->currentUser, $this->l, + $this->config, ])->setMethods(['formatShare']) ->getMock(); @@ -2678,7 +2691,8 @@ public function getOcsDisabledAPI() { $this->rootFolder, $this->urlGenerator, $this->currentUser, - $this->l + $this->l, + $this->config ); } From 56c0634acdf0301f7dfdfe29e5475b56e08b7409 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 1 Sep 2017 12:44:32 +0200 Subject: [PATCH 5/8] Improve default share perms section layout --- settings/css/settings.css | 10 +++++++++- settings/templates/panels/admin/filesharing.php | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/settings/css/settings.css b/settings/css/settings.css index 213b3d708db2..d18b2d8aa8f2 100644 --- a/settings/css/settings.css +++ b/settings/css/settings.css @@ -483,16 +483,24 @@ table.grid td.date{ #shareAPI p { padding-bottom: 0.8em; } #shareAPI input#shareapiExpireAfterNDays {width: 25px;} +#shareAPI .nocheckbox { + padding-left: 20px; +} #shareAPI .indent { padding-left: 28px; } #shareAPI .double-indent { padding-left: 56px; } +#shareAPI #fileSharingSettings h2 { display: inline-block; } +#shareApiDefaultPermissionsSection label { + margin-right: 20px; +} + /* correctly display help icons next to headings */ .icon-info { padding: 11px 20px; @@ -678,4 +686,4 @@ li > a.icon-settings { div.app-settings .section { margin-bottom: 0px; -} \ No newline at end of file +} diff --git a/settings/templates/panels/admin/filesharing.php b/settings/templates/panels/admin/filesharing.php index 7619b03ef873..6f91adafb7f7 100644 --- a/settings/templates/panels/admin/filesharing.php +++ b/settings/templates/panels/admin/filesharing.php @@ -59,7 +59,7 @@ value="1" />

-

+

t('Default local share permissions'));?> @@ -68,7 +68,7 @@ /> -
+

From 7674147b4e6bb829e9b9927c631283e088cbe16c Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 1 Sep 2017 12:51:49 +0200 Subject: [PATCH 6/8] Fix share capabilities JS tests --- core/js/tests/specs/shareitemmodelSpec.js | 25 ++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/core/js/tests/specs/shareitemmodelSpec.js b/core/js/tests/specs/shareitemmodelSpec.js index 791c9e07e192..5263768ce680 100644 --- a/core/js/tests/specs/shareitemmodelSpec.js +++ b/core/js/tests/specs/shareitemmodelSpec.js @@ -25,6 +25,7 @@ describe('OC.Share.ShareItemModel', function() { var fetchSharesDeferred, fetchReshareDeferred; var fileInfoModel, configModel, model; var oldCurrentUser; + var capsSpec; beforeEach(function() { oldCurrentUser = OC.currentUser; @@ -56,8 +57,15 @@ describe('OC.Share.ShareItemModel', function() { configModel: configModel, fileInfoModel: fileInfoModel }); + capsSpec = sinon.stub(OC, 'getCapabilities'); + capsSpec.returns({ + 'files_sharing': { + 'default_permissions': OC.PERMISSION_ALL + } + }); }); afterEach(function() { + capsSpec.restore(); if (fetchSharesStub) { fetchSharesStub.restore(); } @@ -559,7 +567,22 @@ describe('OC.Share.ShareItemModel', function() { }); expect( testWithPermissions(OC.PERMISSION_UPDATE | OC.PERMISSION_SHARE) - ).toEqual(OC.PERMISSION_READ | OC.PERMISSION_UPDATE | OC.PERMISSION_UPDATE); + ).toEqual(OC.PERMISSION_READ | OC.PERMISSION_UPDATE); + }); + it('uses default permissions from capabilities', function() { + capsSpec.returns({ + 'files_sharing': { + 'default_permissions': OC.PERMISSION_READ | OC.PERMISSION_CREATE | OC.PERMISSION_SHARE + } + }); + configModel.set('isResharingAllowed', true); + model.set({ + reshare: {}, + shares: [] + }); + expect( + testWithPermissions(OC.PERMISSION_ALL) + ).toEqual(OC.PERMISSION_READ | OC.PERMISSION_CREATE | OC.PERMISSION_SHARE); }); }); }); From d9178c8de8346aa158bcf5704ccaf48c6c14a285 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 1 Sep 2017 14:53:22 +0200 Subject: [PATCH 7/8] Fix share API unit tests due to default perms setting --- apps/files_sharing/tests/ApiTest.php | 3 ++- tests/Settings/Panels/Admin/FileSharingTest.php | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 1144928d4600..cedeb0fe3e61 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -123,7 +123,8 @@ private function createOCS($request, $userId) { \OC::$server->getRootFolder(), \OC::$server->getURLGenerator(), $currentUser, - $l + $l, + \OC::$server->getConfig() ); } diff --git a/tests/Settings/Panels/Admin/FileSharingTest.php b/tests/Settings/Panels/Admin/FileSharingTest.php index 0e1b5df6ee09..2c0de1399c0d 100644 --- a/tests/Settings/Panels/Admin/FileSharingTest.php +++ b/tests/Settings/Panels/Admin/FileSharingTest.php @@ -13,6 +13,7 @@ use OC\Settings\Panels\Admin\FileSharing; use OC\Settings\Panels\Helper; use OCP\IConfig; +use OCP\IL10N; /** * @package Tests\Settings\Panels\Admin @@ -30,7 +31,8 @@ public function setUp() { parent::setUp(); $this->config = $this->getMockBuilder(IConfig::class)->getMock(); $this->helper = $this->getMockBuilder(Helper::class)->getMock(); - $this->panel = new FileSharing($this->config, $this->helper); + $l10n = $this->getMockBuilder(IL10N::class)->getMock(); + $this->panel = new FileSharing($this->config, $this->helper, $l10n); } public function testGetSection() { From 9e60e862a1efe372a9dff221616d9c514fbf5cbb Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 4 Sep 2017 15:18:13 +0200 Subject: [PATCH 8/8] Tweak layout and wording of defualt file share permissions --- settings/Panels/Admin/FileSharing.php | 2 +- .../templates/panels/admin/filesharing.php | 24 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/settings/Panels/Admin/FileSharing.php b/settings/Panels/Admin/FileSharing.php index aa509107e03d..b8711a16ffd1 100644 --- a/settings/Panels/Admin/FileSharing.php +++ b/settings/Panels/Admin/FileSharing.php @@ -77,7 +77,7 @@ public function getPanel() { ], [ 'id' => 'canupdate', - 'label' => $this->l->t('Update'), + 'label' => $this->l->t('Change'), 'value' => \OCP\Constants::PERMISSION_UPDATE ], [ diff --git a/settings/templates/panels/admin/filesharing.php b/settings/templates/panels/admin/filesharing.php index 6f91adafb7f7..5d4574fd2314 100644 --- a/settings/templates/panels/admin/filesharing.php +++ b/settings/templates/panels/admin/filesharing.php @@ -59,18 +59,6 @@ value="1" />

-

- - t('Default local share permissions'));?> -

-

- - /> - - -

/> @@ -101,4 +89,16 @@ class="noautosave checkbox" value="" />

+

+ + t('Default user and group share permissions'));?> +

+

+ + /> + + +