From 1615312bf1044dcbd98c7e4739467314ada618cf Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 1 Dec 2017 11:35:01 +0100 Subject: add share permissions to settings page Signed-off-by: Bjoern Schiessle --- apps/files_sharing/lib/Capabilities.php | 2 ++ 1 file changed, 2 insertions(+) (limited to 'apps/files_sharing') diff --git a/apps/files_sharing/lib/Capabilities.php b/apps/files_sharing/lib/Capabilities.php index af41add250c..ce10c8df8a1 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 -- cgit v1.2.3 From ebb15283a60d1c2ee5fb731feca1c68275d58264 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 1 Dec 2017 14:49:35 +0100 Subject: 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(-) (limited to 'apps/files_sharing') diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 990571b778f..1e121d8c868 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) { -- cgit v1.2.3 From 3285becdc16a9eb57a507e6ad58100d392f2894a Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 1 Dec 2017 15:50:29 +0100 Subject: fix unit tests Signed-off-by: Bjoern Schiessle --- apps/files_sharing/tests/ApiTest.php | 5 +- .../tests/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(-) (limited to 'apps/files_sharing') diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 6d8e2dd8d84..d7bc169bf4a 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 c438dac2521..a475474e3cf 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 7b60efdc67b..dfc0b11478b 100644 --- a/lib/private/Settings/Admin/Sharing.php +++ b/lib/private/Settings/Admin/Sharing.php @@ -57,29 +57,6 @@ 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'), @@ -98,12 +75,42 @@ class Sharing implements ISettings { '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, + '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'), + '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 + ], + ]; + } + /** * @return string the section ID, e.g. 'sharing' */ diff --git a/tests/lib/Settings/Admin/SharingTest.php b/tests/lib/Settings/Admin/SharingTest.php index ee60979b96d..79065fb8d21 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', []) ], '' ); -- cgit v1.2.3