From 7d0102bf7302a483209e0d1c926260713f0e56c6 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 1 Dec 2017 10:36:13 +0100 Subject: [PATCH 1/7] expose capabilities in js Signed-off-by: Bjoern Schiessle --- core/Controller/OCJSController.php | 8 ++++++-- core/js/js.js | 19 +++++++++++++++++++ lib/private/Template/JSConfigHelper.php | 12 +++++++++++- lib/private/TemplateLayout.php | 3 ++- 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/core/Controller/OCJSController.php b/core/Controller/OCJSController.php index 8db26dd3d3..37fe17c8dd 100644 --- a/core/Controller/OCJSController.php +++ b/core/Controller/OCJSController.php @@ -26,6 +26,7 @@ namespace OC\Core\Controller; use bantu\IniGetWrapper\IniGetWrapper; +use OC\CapabilitiesManager; use OC\Template\JSConfigHelper; use OCP\App\IAppManager; use OCP\AppFramework\Controller; @@ -59,6 +60,7 @@ class OCJSController extends Controller { * @param IGroupManager $groupManager * @param IniGetWrapper $iniWrapper * @param IURLGenerator $urlGenerator + * @param CapabilitiesManager $capabilitiesManager */ public function __construct($appName, IRequest $request, @@ -70,7 +72,8 @@ class OCJSController extends Controller { IConfig $config, IGroupManager $groupManager, IniGetWrapper $iniWrapper, - IURLGenerator $urlGenerator) { + IURLGenerator $urlGenerator, + CapabilitiesManager $capabilitiesManager) { parent::__construct($appName, $request); $this->helper = new JSConfigHelper( @@ -82,7 +85,8 @@ class OCJSController extends Controller { $config, $groupManager, $iniWrapper, - $urlGenerator + $urlGenerator, + $capabilitiesManager ); } diff --git a/core/js/js.js b/core/js/js.js index 9b8829de25..cefe1d92a0 100644 --- a/core/js/js.js +++ b/core/js/js.js @@ -80,6 +80,13 @@ var OCP = {}, */ 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 @@ -308,6 +315,18 @@ var OCP = {}, return OC.webroot; }, + + /** + * Returns the capabilities + * + * @return {array} capabilities + * + * @since 13.0 + */ + getCapabilities: function() { + return OC._capabilities; + }, + /** * Returns the currently logged in user or null if there is no logged in * user (public page mode) diff --git a/lib/private/Template/JSConfigHelper.php b/lib/private/Template/JSConfigHelper.php index b8bff07420..b691a8a64c 100644 --- a/lib/private/Template/JSConfigHelper.php +++ b/lib/private/Template/JSConfigHelper.php @@ -28,6 +28,7 @@ namespace OC\Template; use bantu\IniGetWrapper\IniGetWrapper; +use OC\CapabilitiesManager; use OCP\App\IAppManager; use OCP\Defaults; use OCP\IConfig; @@ -66,6 +67,9 @@ class JSConfigHelper { /** @var IURLGenerator */ private $urlGenerator; + /** @var CapabilitiesManager */ + private $capabilitiesManager; + /** * @param IL10N $l * @param Defaults $defaults @@ -76,6 +80,7 @@ class JSConfigHelper { * @param IGroupManager $groupManager * @param IniGetWrapper $iniWrapper * @param IURLGenerator $urlGenerator + * @param CapabilitiesManager $capabilitiesManager */ public function __construct(IL10N $l, Defaults $defaults, @@ -85,7 +90,8 @@ class JSConfigHelper { IConfig $config, IGroupManager $groupManager, IniGetWrapper $iniWrapper, - IURLGenerator $urlGenerator) { + IURLGenerator $urlGenerator, + CapabilitiesManager $capabilitiesManager) { $this->l = $l; $this->defaults = $defaults; $this->appManager = $appManager; @@ -95,6 +101,7 @@ class JSConfigHelper { $this->groupManager = $groupManager; $this->iniWrapper = $iniWrapper; $this->urlGenerator = $urlGenerator; + $this->capabilitiesManager = $capabilitiesManager; } public function getConfig() { @@ -146,6 +153,8 @@ class JSConfigHelper { $lastConfirmTimestamp = 0; } + $capabilities = $this->capabilitiesManager->getCapabilities(); + $array = [ "oc_debug" => $this->config->getSystemValue('debug', false) ? 'true' : 'false', "oc_isadmin" => $this->groupManager->isAdmin($uid) ? 'true' : 'false', @@ -252,6 +261,7 @@ class JSConfigHelper { 'longFooter' => $this->defaults->getLongFooter(), 'folder' => \OC_Util::getTheme(), ]), + "oc_capabilities" => json_encode($capabilities), ]; if ($this->currentUser !== null) { diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php index 8cc235bf81..115daab70a 100644 --- a/lib/private/TemplateLayout.php +++ b/lib/private/TemplateLayout.php @@ -156,7 +156,8 @@ class TemplateLayout extends \OC_Template { $this->config, \OC::$server->getGroupManager(), \OC::$server->getIniWrapper(), - \OC::$server->getURLGenerator() + \OC::$server->getURLGenerator(), + \OC::$server->getCapabilitiesManager() ); $this->assign('inline_ocjs', $jsConfigHelper->getConfig()); } else { From 1615312bf1044dcbd98c7e4739467314ada618cf Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 1 Dec 2017 11:35:01 +0100 Subject: [PATCH 2/7] add share permissions to settings page Signed-off-by: Bjoern Schiessle --- apps/files_sharing/lib/Capabilities.php | 2 + core/js/shareitemmodel.js | 17 ++--- lib/private/Settings/Admin/Sharing.php | 63 ++++++++++++++----- lib/private/Settings/Manager.php | 2 +- settings/css/settings.scss | 7 +++ settings/js/admin.js | 22 +++++++ settings/templates/settings/admin/sharing.php | 12 ++++ tests/lib/Settings/Admin/SharingTest.php | 7 ++- tests/lib/Settings/ManagerTest.php | 2 +- 9 files changed, 107 insertions(+), 27 deletions(-) diff --git a/apps/files_sharing/lib/Capabilities.php b/apps/files_sharing/lib/Capabilities.php index af41add250..ce10c8df8a 100644 --- a/apps/files_sharing/lib/Capabilities.php +++ b/apps/files_sharing/lib/Capabilities.php @@ -23,6 +23,7 @@ namespace OCA\Files_Sharing; use OCP\Capabilities\ICapability; +use OCP\Constants; use \OCP\IConfig; /** @@ -86,6 +87,7 @@ class Capabilities implements ICapability { $res['group'] = []; $res['group']['enabled'] = $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes') === 'yes'; $res['group']['expire_date']['enabled'] = true; + $res['default_permissions'] = (int)$this->config->getAppValue('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL); } //Federated sharing diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js index b699513c73..e7824aca33 100644 --- a/core/js/shareitemmodel.js +++ b/core/js/shareitemmodel.js @@ -158,23 +158,24 @@ var shareType = attributes.shareType; attributes = _.extend({}, attributes); - // Default permissions are Edit (CRUD) and Share - // Check if these permissions are possible - var permissions = OC.PERMISSION_READ; + // get default permissions + var defaultPermissions = OC.getCapabilities()['files_sharing']['default_permissions'] || OC.PERMISSION_ALL; + 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/lib/private/Settings/Admin/Sharing.php b/lib/private/Settings/Admin/Sharing.php index dbdacf78da..7b60efdc67 100644 --- a/lib/private/Settings/Admin/Sharing.php +++ b/lib/private/Settings/Admin/Sharing.php @@ -28,7 +28,9 @@ namespace OC\Settings\Admin; use OC\Share\Share; use OCP\AppFramework\Http\TemplateResponse; +use OCP\Constants; use OCP\IConfig; +use OCP\IL10N; use OCP\Settings\ISettings; use OCP\Util; @@ -36,11 +38,15 @@ class Sharing implements ISettings { /** @var IConfig */ private $config; + /** @var IL10N */ + private $l; + /** * @param IConfig $config */ - public function __construct(IConfig $config) { + public function __construct(IConfig $config, IL10N $l) { $this->config = $config; + $this->l = $l; } /** @@ -51,23 +57,48 @@ class Sharing implements ISettings { $excludeGroupsList = !is_null(json_decode($excludedGroups)) ? implode('|', json_decode($excludedGroups, true)) : ''; + $permList = [ + [ + 'id' => 'cancreate', + 'label' => $this->l->t('Create'), + 'value' => Constants::PERMISSION_CREATE + ], + [ + 'id' => 'canupdate', + 'label' => $this->l->t('Change'), + 'value' => Constants::PERMISSION_UPDATE + ], + [ + 'id' => 'candelete', + 'label' => $this->l->t('Delete'), + 'value' => Constants::PERMISSION_DELETE + ], + [ + 'id' => 'canshare', + 'label' => $this->l->t('Share'), + 'value' => Constants::PERMISSION_SHARE + ], + ]; + $parameters = [ // Built-In Sharing - 'allowGroupSharing' => $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes'), - 'allowLinks' => $this->config->getAppValue('core', 'shareapi_allow_links', 'yes'), - 'allowPublicUpload' => $this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes'), - 'allowResharing' => $this->config->getAppValue('core', 'shareapi_allow_resharing', 'yes'), - 'allowShareDialogUserEnumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'), - 'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(), - 'onlyShareWithGroupMembers' => Share::shareWithGroupMembersOnly(), - 'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'), - 'shareDefaultExpireDateSet' => $this->config->getAppValue('core', 'shareapi_default_expire_date', 'no'), - 'shareExpireAfterNDays' => $this->config->getAppValue('core', 'shareapi_expire_after_n_days', '7'), - 'shareEnforceExpireDate' => $this->config->getAppValue('core', 'shareapi_enforce_expire_date', 'no'), - 'shareExcludeGroups' => $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes', - 'shareExcludedGroupsList' => $excludeGroupsList, - 'publicShareDisclaimerText' => $this->config->getAppValue('core', 'shareapi_public_link_disclaimertext', null), - 'enableLinkPasswordByDefault' => $this->config->getAppValue('core', 'shareapi_enable_link_password_by_default', 'no'), + 'allowGroupSharing' => $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes'), + 'allowLinks' => $this->config->getAppValue('core', 'shareapi_allow_links', 'yes'), + 'allowPublicUpload' => $this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes'), + 'allowResharing' => $this->config->getAppValue('core', 'shareapi_allow_resharing', 'yes'), + 'allowShareDialogUserEnumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'), + 'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(), + 'onlyShareWithGroupMembers' => Share::shareWithGroupMembersOnly(), + 'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'), + 'shareDefaultExpireDateSet' => $this->config->getAppValue('core', 'shareapi_default_expire_date', 'no'), + 'shareExpireAfterNDays' => $this->config->getAppValue('core', 'shareapi_expire_after_n_days', '7'), + 'shareEnforceExpireDate' => $this->config->getAppValue('core', 'shareapi_enforce_expire_date', 'no'), + 'shareExcludeGroups' => $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes', + 'shareExcludedGroupsList' => $excludeGroupsList, + 'publicShareDisclaimerText' => $this->config->getAppValue('core', 'shareapi_public_link_disclaimertext', null), + 'enableLinkPasswordByDefault' => $this->config->getAppValue('core', 'shareapi_enable_link_password_by_default', 'no'), + 'shareApiDefaultPermissions' => $this->config->getAppValue('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL), + 'shareApiDefaultPermissionsCheckboxes' => $permList, ]; return new TemplateResponse('settings', 'settings/admin/sharing', $parameters, ''); diff --git a/lib/private/Settings/Manager.php b/lib/private/Settings/Manager.php index 387460852a..7111511eca 100644 --- a/lib/private/Settings/Manager.php +++ b/lib/private/Settings/Manager.php @@ -271,7 +271,7 @@ class Manager implements IManager { } if ($section === 'sharing') { /** @var ISettings $form */ - $form = new Admin\Sharing($this->config); + $form = new Admin\Sharing($this->config, $this->l); $forms[$form->getPriority()] = [$form]; } if ($section === 'additional') { diff --git a/settings/css/settings.scss b/settings/css/settings.scss index 05d62423d1..611961b106 100644 --- a/settings/css/settings.scss +++ b/settings/css/settings.scss @@ -1030,6 +1030,13 @@ table.grid td.date { .double-indent { padding-left: 56px; } + .nocheckbox { + padding-left: 20px; + } +} + +#shareApiDefaultPermissionsSection label { + margin-right: 20px; } #fileSharingSettings h3 { diff --git a/settings/js/admin.js b/settings/js/admin.js index 177f6d3f7f..f637cb6459 100644 --- a/settings/js/admin.js +++ b/settings/js/admin.js @@ -121,6 +121,28 @@ $(document).ready(function(){ } }); + $('#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; + }); + var savePublicShareDisclaimerText = _.debounce(function(value) { var options = { success: function() { diff --git a/settings/templates/settings/admin/sharing.php b/settings/templates/settings/admin/sharing.php index 156e8ddd81..1f8e72910c 100644 --- a/settings/templates/settings/admin/sharing.php +++ b/settings/templates/settings/admin/sharing.php @@ -78,6 +78,18 @@ value="1" />

+

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

+

+ + /> + + +

/> diff --git a/tests/lib/Settings/Admin/SharingTest.php b/tests/lib/Settings/Admin/SharingTest.php index 9498a1466d..ee60979b96 100644 --- a/tests/lib/Settings/Admin/SharingTest.php +++ b/tests/lib/Settings/Admin/SharingTest.php @@ -26,6 +26,7 @@ namespace Test\Settings\Admin; use OC\Settings\Admin\Sharing; use OCP\AppFramework\Http\TemplateResponse; use OCP\IConfig; +use OCP\IL10N; use Test\TestCase; class SharingTest extends TestCase { @@ -33,13 +34,17 @@ class SharingTest extends TestCase { private $admin; /** @var IConfig */ private $config; + /** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */ + private $l10n; public function setUp() { parent::setUp(); $this->config = $this->getMockBuilder(IConfig::class)->getMock(); + $this->l10n = $this->getMockBuilder(IL10N::class)->getMock(); $this->admin = new Sharing( - $this->config + $this->config, + $this->l10n ); } diff --git a/tests/lib/Settings/ManagerTest.php b/tests/lib/Settings/ManagerTest.php index 577abc7915..b218a347e8 100644 --- a/tests/lib/Settings/ManagerTest.php +++ b/tests/lib/Settings/ManagerTest.php @@ -209,7 +209,7 @@ class ManagerTest extends TestCase { public function testGetAdminSettings() { $this->assertEquals([ - 0 => [new Sharing($this->config)], + 0 => [new Sharing($this->config, $this->l10n)], ], $this->manager->getAdminSettings('sharing')); } From 7741429229041a4f1b28230fd039b2dc9ec2dada Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 1 Dec 2017 14:15:05 +0100 Subject: [PATCH 3/7] improved layout of the admin settings Signed-off-by: Bjoern Schiessle --- settings/templates/settings/admin/sharing.php | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/settings/templates/settings/admin/sharing.php b/settings/templates/settings/admin/sharing.php index 1f8e72910c..5913524be4 100644 --- a/settings/templates/settings/admin/sharing.php +++ b/settings/templates/settings/admin/sharing.php @@ -78,18 +78,6 @@ value="1" />

-

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

-

- - /> - - -

/> @@ -118,4 +106,15 @@

+ +

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

+ +

+ + /> + + +

From ebb15283a60d1c2ee5fb731feca1c68275d58264 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 1 Dec 2017 14:49:35 +0100 Subject: [PATCH 4/7] share api: use default permission of no permission is given Signed-off-by: Bjoern Schiessle --- .../lib/Controller/ShareAPIController.php | 63 ++++++++++--------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 990571b778..1e121d8c86 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -35,8 +35,10 @@ use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSForbiddenException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; +use OCP\Constants; use OCP\Files\Node; use OCP\Files\NotFoundException; +use OCP\IConfig; use OCP\IGroupManager; use OCP\IL10N; use OCP\IUserManager; @@ -75,6 +77,8 @@ class ShareAPIController extends OCSController { private $l; /** @var \OCP\Files\Node */ private $lockedNode; + /** @var IConfig */ + private $config; /** * Share20OCS constructor. @@ -88,6 +92,7 @@ class ShareAPIController extends OCSController { * @param IURLGenerator $urlGenerator * @param string $userId * @param IL10N $l10n + * @param IConfig $config */ public function __construct( $appName, @@ -98,7 +103,8 @@ class ShareAPIController extends OCSController { IRootFolder $rootFolder, IURLGenerator $urlGenerator, $userId, - IL10N $l10n + IL10N $l10n, + IConfig $config ) { parent::__construct($appName, $request); @@ -110,6 +116,7 @@ class ShareAPIController extends OCSController { $this->urlGenerator = $urlGenerator; $this->currentUser = $userId; $this->l = $l10n; + $this->config = $config; } /** @@ -318,7 +325,7 @@ class ShareAPIController extends OCSController { */ public function createShare( $path = null, - $permissions = \OCP\Constants::PERMISSION_ALL, + $permissions = null, $shareType = -1, $shareWith = null, $publicUpload = 'false', @@ -327,6 +334,10 @@ class ShareAPIController extends OCSController { ) { $share = $this->shareManager->newShare(); + if ($permissions === null) { + $permissions = $this->config->getAppValue('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL); + } + // Verify path if ($path === null) { throw new OCSNotFoundException($this->l->t('Please specify a file or folder path')); @@ -347,17 +358,17 @@ class ShareAPIController extends OCSController { throw new OCSNotFoundException($this->l->t('Could not create share')); } - if ($permissions < 0 || $permissions > \OCP\Constants::PERMISSION_ALL) { + if ($permissions < 0 || $permissions > Constants::PERMISSION_ALL) { throw new OCSNotFoundException($this->l->t('invalid permissions')); } // Shares always require read permissions - $permissions |= \OCP\Constants::PERMISSION_READ; + $permissions |= Constants::PERMISSION_READ; if ($path instanceof \OCP\Files\File) { // Single file shares should never have delete or create permissions - $permissions &= ~\OCP\Constants::PERMISSION_DELETE; - $permissions &= ~\OCP\Constants::PERMISSION_CREATE; + $permissions &= ~Constants::PERMISSION_DELETE; + $permissions &= ~Constants::PERMISSION_CREATE; } /* @@ -414,13 +425,13 @@ class ShareAPIController extends OCSController { } $share->setPermissions( - \OCP\Constants::PERMISSION_READ | - \OCP\Constants::PERMISSION_CREATE | - \OCP\Constants::PERMISSION_UPDATE | - \OCP\Constants::PERMISSION_DELETE + Constants::PERMISSION_READ | + Constants::PERMISSION_CREATE | + Constants::PERMISSION_UPDATE | + Constants::PERMISSION_DELETE ); } else { - $share->setPermissions(\OCP\Constants::PERMISSION_READ); + $share->setPermissions(Constants::PERMISSION_READ); } // Set password @@ -447,13 +458,9 @@ class ShareAPIController extends OCSController { $share->setPermissions($permissions); } else if ($shareType === \OCP\Share::SHARE_TYPE_EMAIL) { if ($share->getNodeType() === 'file') { - $share->setPermissions(\OCP\Constants::PERMISSION_READ); + $share->setPermissions(Constants::PERMISSION_READ); } else { - $share->setPermissions( - \OCP\Constants::PERMISSION_READ | - \OCP\Constants::PERMISSION_CREATE | - \OCP\Constants::PERMISSION_UPDATE | - \OCP\Constants::PERMISSION_DELETE); + $share->setPermissions($permissions); } $share->setSharedWith($shareWith); } else if ($shareType === \OCP\Share::SHARE_TYPE_CIRCLE) { @@ -698,23 +705,23 @@ class ShareAPIController extends OCSController { $newPermissions = null; if ($publicUpload === 'true') { - $newPermissions = \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE; + $newPermissions = Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE; } else if ($publicUpload === 'false') { - $newPermissions = \OCP\Constants::PERMISSION_READ; + $newPermissions = Constants::PERMISSION_READ; } if ($permissions !== null) { $newPermissions = (int)$permissions; - $newPermissions = $newPermissions & ~\OCP\Constants::PERMISSION_SHARE; + $newPermissions = $newPermissions & ~Constants::PERMISSION_SHARE; } if ($newPermissions !== null && !in_array($newPermissions, [ - \OCP\Constants::PERMISSION_READ, - \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE, // legacy - \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE, // correct - \OCP\Constants::PERMISSION_CREATE, // hidden file list - \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE, // allow to edit single files + Constants::PERMISSION_READ, + Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE, // legacy + Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE, // correct + Constants::PERMISSION_CREATE, // hidden file list + Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE, // allow to edit single files ]) ) { throw new OCSBadRequestException($this->l->t('Can\'t change permissions for public share links')); @@ -722,9 +729,9 @@ class ShareAPIController extends OCSController { if ( // legacy - $newPermissions === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE) || + $newPermissions === (Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE) || // correct - $newPermissions === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE) + $newPermissions === (Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE) ) { if (!$this->shareManager->shareApiLinkAllowPublicUpload()) { throw new OCSForbiddenException($this->l->t('Public upload disabled by the administrator')); @@ -735,7 +742,7 @@ class ShareAPIController extends OCSController { } // normalize to correct public upload permissions - $newPermissions = \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE; + $newPermissions = Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE; } if ($newPermissions !== null) { From 3285becdc16a9eb57a507e6ad58100d392f2894a Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 1 Dec 2017 15:50:29 +0100 Subject: [PATCH 5/7] fix unit tests Signed-off-by: Bjoern Schiessle --- apps/files_sharing/tests/ApiTest.php | 5 +- .../Controller/ShareAPIControllerTest.php | 13 ++++- lib/private/Settings/Admin/Sharing.php | 55 +++++++++++-------- tests/lib/Settings/Admin/SharingTest.php | 20 ++++++- 4 files changed, 65 insertions(+), 28 deletions(-) diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 6d8e2dd8d8..d7bc169bf4 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -37,6 +37,7 @@ use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSForbiddenException; use OCP\AppFramework\OCS\OCSNotFoundException; +use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; @@ -105,6 +106,7 @@ class ApiTest extends TestCase { ->will($this->returnCallback(function($text, $parameters = []) { return vsprintf($text, $parameters); })); + $config = $this->createMock(IConfig::class); return new ShareAPIController( self::APP_NAME, @@ -115,7 +117,8 @@ class ApiTest extends TestCase { \OC::$server->getRootFolder(), \OC::$server->getURLGenerator(), $userId, - $l + $l, + $config ); } diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index c438dac252..a475474e3c 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -31,6 +31,7 @@ use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\Storage; +use OCP\IConfig; use OCP\IL10N; use OCA\Files_Sharing\Controller\ShareAPIController; use OCP\Files\NotFoundException; @@ -84,6 +85,9 @@ class ShareAPIControllerTest extends TestCase { /** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */ private $l; + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + private $config; + protected function setUp() { $this->shareManager = $this->createMock(IManager::class); $this->shareManager @@ -102,6 +106,7 @@ class ShareAPIControllerTest extends TestCase { ->will($this->returnCallback(function($text, $parameters = []) { return vsprintf($text, $parameters); })); + $this->config = $this->createMock(IConfig::class); $this->ocs = new ShareAPIController( $this->appName, @@ -112,7 +117,8 @@ class ShareAPIControllerTest extends TestCase { $this->rootFolder, $this->urlGenerator, $this->currentUser, - $this->l + $this->l, + $this->config ); } @@ -131,6 +137,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, + $this->config ])->setMethods(['formatShare']) ->getMock(); } @@ -439,6 +446,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, + $this->config ])->setMethods(['canAccessShare']) ->getMock(); @@ -707,6 +715,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, + $this->config ])->setMethods(['formatShare']) ->getMock(); @@ -804,6 +813,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, + $this->config ])->setMethods(['formatShare']) ->getMock(); @@ -1119,6 +1129,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->currentUser, $this->l, + $this->config ])->setMethods(['formatShare']) ->getMock(); diff --git a/lib/private/Settings/Admin/Sharing.php b/lib/private/Settings/Admin/Sharing.php index 7b60efdc67..dfc0b11478 100644 --- a/lib/private/Settings/Admin/Sharing.php +++ b/lib/private/Settings/Admin/Sharing.php @@ -57,7 +57,37 @@ class Sharing implements ISettings { $excludeGroupsList = !is_null(json_decode($excludedGroups)) ? implode('|', json_decode($excludedGroups, true)) : ''; - $permList = [ + $parameters = [ + // Built-In Sharing + 'allowGroupSharing' => $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes'), + 'allowLinks' => $this->config->getAppValue('core', 'shareapi_allow_links', 'yes'), + 'allowPublicUpload' => $this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes'), + 'allowResharing' => $this->config->getAppValue('core', 'shareapi_allow_resharing', 'yes'), + 'allowShareDialogUserEnumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'), + 'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(), + 'onlyShareWithGroupMembers' => Share::shareWithGroupMembersOnly(), + 'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'), + 'shareDefaultExpireDateSet' => $this->config->getAppValue('core', 'shareapi_default_expire_date', 'no'), + 'shareExpireAfterNDays' => $this->config->getAppValue('core', 'shareapi_expire_after_n_days', '7'), + 'shareEnforceExpireDate' => $this->config->getAppValue('core', 'shareapi_enforce_expire_date', 'no'), + 'shareExcludeGroups' => $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes', + 'shareExcludedGroupsList' => $excludeGroupsList, + 'publicShareDisclaimerText' => $this->config->getAppValue('core', 'shareapi_public_link_disclaimertext', null), + 'enableLinkPasswordByDefault' => $this->config->getAppValue('core', 'shareapi_enable_link_password_by_default', 'no'), + 'shareApiDefaultPermissions' => $this->config->getAppValue('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL), + 'shareApiDefaultPermissionsCheckboxes' => $this->getSharePermissionList(), + ]; + + return new TemplateResponse('settings', 'settings/admin/sharing', $parameters, ''); + } + + /** + * get share permission list for template + * + * @return array + */ + private function getSharePermissionList() { + return [ [ 'id' => 'cancreate', 'label' => $this->l->t('Create'), @@ -79,29 +109,6 @@ class Sharing implements ISettings { 'value' => Constants::PERMISSION_SHARE ], ]; - - $parameters = [ - // Built-In Sharing - 'allowGroupSharing' => $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes'), - 'allowLinks' => $this->config->getAppValue('core', 'shareapi_allow_links', 'yes'), - 'allowPublicUpload' => $this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes'), - 'allowResharing' => $this->config->getAppValue('core', 'shareapi_allow_resharing', 'yes'), - 'allowShareDialogUserEnumeration' => $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes'), - 'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(), - 'onlyShareWithGroupMembers' => Share::shareWithGroupMembersOnly(), - 'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'), - 'shareDefaultExpireDateSet' => $this->config->getAppValue('core', 'shareapi_default_expire_date', 'no'), - 'shareExpireAfterNDays' => $this->config->getAppValue('core', 'shareapi_expire_after_n_days', '7'), - 'shareEnforceExpireDate' => $this->config->getAppValue('core', 'shareapi_enforce_expire_date', 'no'), - 'shareExcludeGroups' => $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes', - 'shareExcludedGroupsList' => $excludeGroupsList, - 'publicShareDisclaimerText' => $this->config->getAppValue('core', 'shareapi_public_link_disclaimertext', null), - 'enableLinkPasswordByDefault' => $this->config->getAppValue('core', 'shareapi_enable_link_password_by_default', 'no'), - 'shareApiDefaultPermissions' => $this->config->getAppValue('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL), - 'shareApiDefaultPermissionsCheckboxes' => $permList, - ]; - - return new TemplateResponse('settings', 'settings/admin/sharing', $parameters, ''); } /** diff --git a/tests/lib/Settings/Admin/SharingTest.php b/tests/lib/Settings/Admin/SharingTest.php index ee60979b96..79065fb8d2 100644 --- a/tests/lib/Settings/Admin/SharingTest.php +++ b/tests/lib/Settings/Admin/SharingTest.php @@ -25,6 +25,7 @@ namespace Test\Settings\Admin; use OC\Settings\Admin\Sharing; use OCP\AppFramework\Http\TemplateResponse; +use OCP\Constants; use OCP\IConfig; use OCP\IL10N; use Test\TestCase; @@ -114,6 +115,11 @@ class SharingTest extends TestCase { ->method('getAppValue') ->with('core', 'shareapi_enable_link_password_by_default', 'no') ->willReturn('yes'); + $this->config + ->expects($this->at(13)) + ->method('getAppValue') + ->with('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL) + ->willReturn(Constants::PERMISSION_ALL); $expected = new TemplateResponse( 'settings', @@ -133,7 +139,9 @@ class SharingTest extends TestCase { 'shareExcludeGroups' => false, 'shareExcludedGroupsList' => '', 'publicShareDisclaimerText' => 'Lorem ipsum', - 'enableLinkPasswordByDefault' => 'yes' + 'enableLinkPasswordByDefault' => 'yes', + 'shareApiDefaultPermissions' => Constants::PERMISSION_ALL, + 'shareApiDefaultPermissionsCheckboxes' => $this->invokePrivate($this->admin, 'getSharePermissionList', []) ], '' ); @@ -207,6 +215,12 @@ class SharingTest extends TestCase { ->method('getAppValue') ->with('core', 'shareapi_enable_link_password_by_default', 'no') ->willReturn('yes'); + $this->config + ->expects($this->at(13)) + ->method('getAppValue') + ->with('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL) + ->willReturn(Constants::PERMISSION_ALL); + $expected = new TemplateResponse( 'settings', @@ -226,7 +240,9 @@ class SharingTest extends TestCase { 'shareExcludeGroups' => true, 'shareExcludedGroupsList' => 'NoSharers|OtherNoSharers', 'publicShareDisclaimerText' => 'Lorem ipsum', - 'enableLinkPasswordByDefault' => 'yes' + 'enableLinkPasswordByDefault' => 'yes', + 'shareApiDefaultPermissions' => Constants::PERMISSION_ALL, + 'shareApiDefaultPermissionsCheckboxes' => $this->invokePrivate($this->admin, 'getSharePermissionList', []) ], '' ); From 7466468af1a4c830c7bb825fda141a336d541b0d Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 1 Sep 2017 12:51:49 +0200 Subject: [PATCH 6/7] Fix share capabilities JS tests Signed-off-by: Bjoern Schiessle --- 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 3b17051508..0a345786b7 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(); } @@ -527,7 +535,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 20ec0344a26faf725762e3b344b66bb45ef1a5a2 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 15 Feb 2018 11:06:51 +0100 Subject: [PATCH 7/7] Fix JSDoc Signed-off-by: Morris Jobke --- core/js/js.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/js/js.js b/core/js/js.js index cefe1d92a0..fa92508ff7 100644 --- a/core/js/js.js +++ b/core/js/js.js @@ -321,7 +321,7 @@ var OCP = {}, * * @return {array} capabilities * - * @since 13.0 + * @since 14.0 */ getCapabilities: function() { return OC._capabilities;