diff options
author | Morris Jobke <hey@morrisjobke.de> | 2021-05-21 15:35:39 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-05-21 15:35:39 +0200 |
commit | f1dbabd9109c08e62d6f82124c63201c96ad2203 (patch) | |
tree | ea6b862c1fa97ec5ad03895c120268bd25d1d35e | |
parent | b1ad3faf14140e3c85a7eec1dade88cd551cc747 (diff) | |
parent | 8d7fae8fae054eaa08635fea3ba092196c6f8b84 (diff) | |
download | nextcloud-server-f1dbabd9109c08e62d6f82124c63201c96ad2203.tar.gz nextcloud-server-f1dbabd9109c08e62d6f82124c63201c96ad2203.zip |
Merge pull request #26727 from nextcloud/group-exclude-link-share
Add option to exclude groups from creating link shares
-rw-r--r-- | apps/files/lib/Controller/ViewController.php | 9 | ||||
-rw-r--r-- | apps/files/list.php | 6 | ||||
-rw-r--r-- | apps/files/tests/Controller/ViewControllerTest.php | 7 | ||||
-rw-r--r-- | apps/files_sharing/lib/AppInfo/Application.php | 20 | ||||
-rw-r--r-- | apps/files_sharing/lib/Capabilities.php | 38 | ||||
-rw-r--r-- | apps/files_sharing/tests/CapabilitiesTest.php | 37 | ||||
-rw-r--r-- | apps/settings/js/admin.js | 2 | ||||
-rw-r--r-- | apps/settings/lib/Settings/Admin/Sharing.php | 4 | ||||
-rw-r--r-- | apps/settings/templates/settings/admin/sharing.php | 10 | ||||
-rw-r--r-- | apps/settings/tests/Settings/Admin/SharingTest.php | 6 | ||||
-rw-r--r-- | lib/private/Server.php | 3 | ||||
-rw-r--r-- | lib/private/Share20/Manager.php | 120 | ||||
-rw-r--r-- | lib/private/legacy/OC_Util.php | 40 | ||||
-rw-r--r-- | lib/public/Share/IManager.php | 48 | ||||
-rw-r--r-- | tests/lib/Share20/ManagerTest.php | 33 | ||||
-rw-r--r-- | tests/lib/UtilTest.php | 49 |
16 files changed, 253 insertions, 179 deletions
diff --git a/apps/files/lib/Controller/ViewController.php b/apps/files/lib/Controller/ViewController.php index efaf2dc602f..b055f9a38b5 100644 --- a/apps/files/lib/Controller/ViewController.php +++ b/apps/files/lib/Controller/ViewController.php @@ -55,6 +55,7 @@ use OCP\IL10N; use OCP\IRequest; use OCP\IURLGenerator; use OCP\IUserSession; +use OCP\Share\IManager; /** * Class ViewController @@ -86,6 +87,8 @@ class ViewController extends Controller { private $initialState; /** @var ITemplateManager */ private $templateManager; + /** @var IManager */ + private $shareManager; public function __construct(string $appName, IRequest $request, @@ -98,7 +101,8 @@ class ViewController extends Controller { IRootFolder $rootFolder, Helper $activityHelper, IInitialState $initialState, - ITemplateManager $templateManager + ITemplateManager $templateManager, + IManager $shareManager ) { parent::__construct($appName, $request); $this->appName = $appName; @@ -113,6 +117,7 @@ class ViewController extends Controller { $this->activityHelper = $activityHelper; $this->initialState = $initialState; $this->templateManager = $templateManager; + $this->shareManager = $shareManager; } /** @@ -302,7 +307,7 @@ class ViewController extends Controller { $params['owner'] = $storageInfo['owner'] ?? ''; $params['ownerDisplayName'] = $storageInfo['ownerDisplayName'] ?? ''; $params['isPublic'] = false; - $params['allowShareWithLink'] = $this->config->getAppValue('core', 'shareapi_allow_links', 'yes'); + $params['allowShareWithLink'] = $this->shareManager->shareApiAllowLinks() ? 'yes' : 'no'; $params['defaultFileSorting'] = $this->config->getUserValue($user, 'files', 'file_sorting', 'name'); $params['defaultFileSortingDirection'] = $this->config->getUserValue($user, 'files', 'file_sorting_direction', 'asc'); $params['showgridview'] = $this->config->getUserValue($user, 'files', 'show_grid', false); diff --git a/apps/files/list.php b/apps/files/list.php index 52d736516c9..308d149c873 100644 --- a/apps/files/list.php +++ b/apps/files/list.php @@ -22,10 +22,14 @@ * */ +use OCP\Share\IManager; + $config = \OC::$server->getConfig(); $userSession = \OC::$server->getUserSession(); // TODO: move this to the generated config.js -$publicUploadEnabled = $config->getAppValue('core', 'shareapi_allow_public_upload', 'yes'); +/** @var IManager $shareManager */ +$shareManager = \OC::$server->get(IManager::class); +$publicUploadEnabled = $shareManager->shareApiLinkAllowPublicUpload() ? 'yes' : 'no';; $showgridview = $config->getUserValue($userSession->getUser()->getUID(), 'files', 'show_grid', false); $isIE = OC_Util::isIe(); diff --git a/apps/files/tests/Controller/ViewControllerTest.php b/apps/files/tests/Controller/ViewControllerTest.php index bc233599e34..3453914b27a 100644 --- a/apps/files/tests/Controller/ViewControllerTest.php +++ b/apps/files/tests/Controller/ViewControllerTest.php @@ -48,6 +48,7 @@ use OCP\IRequest; use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserSession; +use OCP\Share\IManager; use OCP\Template; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Test\TestCase; @@ -84,6 +85,8 @@ class ViewControllerTest extends TestCase { private $initialState; /** @var ITemplateManager|\PHPUnit\Framework\MockObject\MockObject */ private $templateManager; + /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ + private $shareManager; protected function setUp(): void { parent::setUp(); @@ -105,6 +108,7 @@ class ViewControllerTest extends TestCase { $this->activityHelper = $this->createMock(Helper::class); $this->initialState = $this->createMock(IInitialState::class); $this->templateManager = $this->createMock(ITemplateManager::class); + $this->shareManager = $this->createMock(IManager::class); $this->viewController = $this->getMockBuilder('\OCA\Files\Controller\ViewController') ->setConstructorArgs([ 'files', @@ -119,6 +123,7 @@ class ViewControllerTest extends TestCase { $this->activityHelper, $this->initialState, $this->templateManager, + $this->shareManager, ]) ->setMethods([ 'getStorageInfo', @@ -153,6 +158,8 @@ class ViewControllerTest extends TestCase { ->expects($this->any()) ->method('getAppValue') ->willReturnArgument(2); + $this->shareManager->method('shareApiAllowLinks') + ->willReturn(true); $nav = new Template('files', 'appnavigation'); $nav->assign('usage_relative', 123); diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index 078a0a5f59d..213e441cdb8 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -56,7 +56,9 @@ use OCP\Group\Events\UserAddedEvent; use OCP\IDBConnection; use OCP\IGroup; use OCP\IServerContainer; +use OCP\IUserSession; use OCP\Share\Events\ShareCreatedEvent; +use OCP\Share\IManager; use OCP\Util; use Psr\Container\ContainerInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -166,20 +168,24 @@ class Application extends App { } protected function setupSharingMenus() { - $config = \OC::$server->getConfig(); + /** @var IManager $shareManager */ + $shareManager = \OC::$server->get(IManager::class); - if ($config->getAppValue('core', 'shareapi_enabled', 'yes') !== 'yes' || !class_exists('\OCA\Files\App')) { + if (!$shareManager->shareApiEnabled() || !class_exists('\OCA\Files\App')) { return; } // show_Quick_Access stored as string - \OCA\Files\App::getNavigationManager()->add(function () { - $config = \OC::$server->getConfig(); + \OCA\Files\App::getNavigationManager()->add(function () use ($shareManager) { $l = \OC::$server->getL10N('files_sharing'); + /** @var IUserSession $userSession */ + $userSession = \OC::$server->get(IUserSession::class); + $user = $userSession->getUser(); + $userId = $user ? $user->getUID() : null; $sharingSublistArray = []; - if (\OCP\Util::isSharingDisabledForUser() === false) { + if ($shareManager->sharingDisabledForUser($userId) === false) { $sharingSublistArray[] = [ 'id' => 'sharingout', 'appname' => 'files_sharing', @@ -197,9 +203,9 @@ class Application extends App { 'name' => $l->t('Shared with you'), ]; - if (\OCP\Util::isSharingDisabledForUser() === false) { + if ($shareManager->sharingDisabledForUser($userId) === false) { // Check if sharing by link is enabled - if ($config->getAppValue('core', 'shareapi_allow_links', 'yes') === 'yes') { + if ($shareManager->shareApiAllowLinks()) { $sharingSublistArray[] = [ 'id' => 'sharinglinks', 'appname' => 'files_sharing', diff --git a/apps/files_sharing/lib/Capabilities.php b/apps/files_sharing/lib/Capabilities.php index 5a0b9d7e71d..dca32d123f4 100644 --- a/apps/files_sharing/lib/Capabilities.php +++ b/apps/files_sharing/lib/Capabilities.php @@ -28,6 +28,7 @@ namespace OCA\Files_Sharing; use OCP\Capabilities\ICapability; use OCP\Constants; use OCP\IConfig; +use OCP\Share\IManager; /** * Class Capabilities @@ -38,9 +39,12 @@ class Capabilities implements ICapability { /** @var IConfig */ private $config; + /** @var IManager */ + private $shareManager; - public function __construct(IConfig $config) { + public function __construct(IConfig $config, IManager $shareManager) { $this->config = $config; + $this->shareManager = $shareManager; } /** @@ -51,7 +55,7 @@ class Capabilities implements ICapability { public function getCapabilities() { $res = []; - if ($this->config->getAppValue('core', 'shareapi_enabled', 'yes') !== 'yes') { + if (!$this->shareManager->shareApiEnabled()) { $res['api_enabled'] = false; $res['public'] = ['enabled' => false]; $res['user'] = ['send_mail' => false]; @@ -60,10 +64,10 @@ class Capabilities implements ICapability { $res['api_enabled'] = true; $public = []; - $public['enabled'] = $this->config->getAppValue('core', 'shareapi_allow_links', 'yes') === 'yes'; + $public['enabled'] = $this->shareManager->shareApiAllowLinks(); if ($public['enabled']) { $public['password'] = []; - $public['password']['enforced'] = ($this->config->getAppValue('core', 'shareapi_enforce_links_password', 'no') === 'yes'); + $public['password']['enforced'] = $this->shareManager->shareApiLinkEnforcePassword(); if ($public['password']['enforced']) { $public['password']['askForOptionalPassword'] = false; @@ -73,28 +77,28 @@ class Capabilities implements ICapability { $public['expire_date'] = []; $public['multiple_links'] = true; - $public['expire_date']['enabled'] = $this->config->getAppValue('core', 'shareapi_default_expire_date', 'no') === 'yes'; + $public['expire_date']['enabled'] = $this->shareManager->shareApiLinkDefaultExpireDate(); if ($public['expire_date']['enabled']) { - $public['expire_date']['days'] = $this->config->getAppValue('core', 'shareapi_expire_after_n_days', '7'); - $public['expire_date']['enforced'] = $this->config->getAppValue('core', 'shareapi_enforce_expire_date', 'no') === 'yes'; + $public['expire_date']['days'] = $this->shareManager->shareApiLinkDefaultExpireDays(); + $public['expire_date']['enforced'] = $this->shareManager->shareApiLinkDefaultExpireDateEnforced(); } $public['expire_date_internal'] = []; - $public['expire_date_internal']['enabled'] = $this->config->getAppValue('core', 'shareapi_default_internal_expire_date', 'no') === 'yes'; + $public['expire_date_internal']['enabled'] = $this->shareManager->shareApiInternalDefaultExpireDate(); if ($public['expire_date_internal']['enabled']) { - $public['expire_date_internal']['days'] = $this->config->getAppValue('core', 'shareapi_internal_expire_after_n_days', '7'); - $public['expire_date_internal']['enforced'] = $this->config->getAppValue('core', 'shareapi_enforce_internal_expire_date', 'no') === 'yes'; + $public['expire_date_internal']['days'] = $this->shareManager->shareApiInternalDefaultExpireDays(); + $public['expire_date_internal']['enforced'] = $this->shareManager->shareApiInternalDefaultExpireDateEnforced(); } $public['expire_date_remote'] = []; - $public['expire_date_remote']['enabled'] = $this->config->getAppValue('core', 'shareapi_default_remote_expire_date', 'no') === 'yes'; + $public['expire_date_remote']['enabled'] = $this->shareManager->shareApiRemoteDefaultExpireDate(); if ($public['expire_date_remote']['enabled']) { - $public['expire_date_remote']['days'] = $this->config->getAppValue('core', 'shareapi_remote_expire_after_n_days', '7'); - $public['expire_date_remote']['enforced'] = $this->config->getAppValue('core', 'shareapi_enforce_remote_expire_date', 'no') === 'yes'; + $public['expire_date_remote']['days'] = $this->shareManager->shareApiRemoteDefaultExpireDays(); + $public['expire_date_remote']['enforced'] = $this->shareManager->shareApiRemoteDefaultExpireDateEnforced(); } $public['send_mail'] = $this->config->getAppValue('core', 'shareapi_allow_public_notification', 'no') === 'yes'; - $public['upload'] = $this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes') === 'yes'; + $public['upload'] = $this->shareManager->shareApiLinkAllowPublicUpload(); $public['upload_files_drop'] = $public['upload']; } $res['public'] = $public; @@ -106,17 +110,17 @@ class Capabilities implements ICapability { // deprecated in favour of 'group', but we need to keep it for now // in order to stay compatible with older clients - $res['group_sharing'] = $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes') === 'yes'; + $res['group_sharing'] = $this->shareManager->allowGroupSharing(); $res['group'] = []; - $res['group']['enabled'] = $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes') === 'yes'; + $res['group']['enabled'] = $this->shareManager->allowGroupSharing(); $res['group']['expire_date']['enabled'] = true; $res['default_permissions'] = (int)$this->config->getAppValue('core', 'shareapi_default_permissions', Constants::PERMISSION_ALL); } //Federated sharing $res['federation'] = [ - 'outgoing' => $this->config->getAppValue('files_sharing', 'outgoing_server2server_share_enabled', 'yes') === 'yes', + 'outgoing' => $this->shareManager->outgoingServer2ServerSharesAllowed(), 'incoming' => $this->config->getAppValue('files_sharing', 'incoming_server2server_share_enabled', 'yes') === 'yes', // old bogus one, expire_date was not working before, keeping for compatibility 'expire_date' => ['enabled' => true], diff --git a/apps/files_sharing/tests/CapabilitiesTest.php b/apps/files_sharing/tests/CapabilitiesTest.php index fbe8c832c1d..7f019603e32 100644 --- a/apps/files_sharing/tests/CapabilitiesTest.php +++ b/apps/files_sharing/tests/CapabilitiesTest.php @@ -27,8 +27,24 @@ namespace OCA\Files_Sharing\Tests; +use OC\Share20\Manager; use OCA\Files_Sharing\Capabilities; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\IRootFolder; +use OCP\Files\Mount\IMountManager; use OCP\IConfig; +use OCP\IGroupManager; +use OCP\IL10N; +use OCP\ILogger; +use OCP\IURLGenerator; +use OCP\IUserManager; +use OCP\IUserSession; +use OCP\L10N\IFactory; +use OCP\Mail\IMailer; +use OCP\Security\IHasher; +use OCP\Security\ISecureRandom; +use OCP\Share\IProviderFactory; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** * Class CapabilitiesTest @@ -60,7 +76,26 @@ class CapabilitiesTest extends \Test\TestCase { private function getResults(array $map) { $config = $this->getMockBuilder(IConfig::class)->disableOriginalConstructor()->getMock(); $config->method('getAppValue')->willReturnMap($map); - $cap = new Capabilities($config); + $shareManager = new Manager( + $this->createMock(ILogger::class), + $config, + $this->createMock(ISecureRandom::class), + $this->createMock(IHasher::class), + $this->createMock(IMountManager::class), + $this->createMock(IGroupManager::class), + $this->createMock(IL10N::class), + $this->createMock(IFactory::class), + $this->createMock(IProviderFactory::class), + $this->createMock(IUserManager::class), + $this->createMock(IRootFolder::class), + $this->createMock(EventDispatcherInterface::class), + $this->createMock(IMailer::class), + $this->createMock(IURLGenerator::class), + $this->createMock(\OC_Defaults::class), + $this->createMock(IEventDispatcher::class), + $this->createMock(IUserSession::class) + ); + $cap = new Capabilities($config, $shareManager); $result = $this->getFilesSharingPart($cap->getCapabilities()); return $result; } diff --git a/apps/settings/js/admin.js b/apps/settings/js/admin.js index 271d13b43d0..f749c9e132c 100644 --- a/apps/settings/js/admin.js +++ b/apps/settings/js/admin.js @@ -1,5 +1,5 @@ window.addEventListener('DOMContentLoaded', function(){ - $('#excludedGroups').each(function (index, element) { + $('#excludedGroups,#linksExcludedGroups').each(function (index, element) { OC.Settings.setupGroupsSelect($(element)); $(element).change(function(ev) { var groups = ev.val || []; diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php index 7eb9649a1aa..8d759b4ae85 100644 --- a/apps/settings/lib/Settings/Admin/Sharing.php +++ b/apps/settings/lib/Settings/Admin/Sharing.php @@ -64,11 +64,15 @@ class Sharing implements ISettings { $excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', ''); $excludeGroupsList = !is_null(json_decode($excludedGroups)) ? implode('|', json_decode($excludedGroups, true)) : ''; + $linksExcludedGroups = $this->config->getAppValue('core', 'shareapi_allow_links_exclude_groups', ''); + $linksExcludeGroupsList = !is_null(json_decode($linksExcludedGroups)) + ? implode('|', json_decode($linksExcludedGroups, true)) : ''; $parameters = [ // Built-In Sharing 'allowGroupSharing' => $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes'), 'allowLinks' => $this->config->getAppValue('core', 'shareapi_allow_links', 'yes'), + 'allowLinksExcludeGroups' => $linksExcludeGroupsList, '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'), diff --git a/apps/settings/templates/settings/admin/sharing.php b/apps/settings/templates/settings/admin/sharing.php index 4bb2ddca7d9..9b562cface6 100644 --- a/apps/settings/templates/settings/admin/sharing.php +++ b/apps/settings/templates/settings/admin/sharing.php @@ -138,6 +138,14 @@ <p class="<?php if ($_['shareAPIEnabled'] === 'no') { p('hidden'); }?>"> + <p class="indent"> + <?php p($l->t('Exclude groups from creating link shares:'));?> + </p> + <p id="selectLinksExcludedGroups" class="indent <?php if ($_['allowLinks'] === 'no') { + p('hidden'); +} ?>"> + <input name="shareapi_allow_links_exclude_groups" type="hidden" id="linksExcludedGroups" value="<?php p($_['allowLinksExcludeGroups']) ?>" style="width: 400px" class="noJSAutoUpdate"/> + </p> <input type="checkbox" name="shareapi_allow_resharing" id="allowResharing" class="checkbox" value="1" <?php if ($_['allowResharing'] === 'yes') { print_unescaped('checked="checked"'); @@ -176,7 +184,7 @@ } ?>"> <input name="shareapi_exclude_groups_list" type="hidden" id="excludedGroups" value="<?php p($_['shareExcludedGroupsList']) ?>" style="width: 400px" class="noJSAutoUpdate"/> <br /> - <em><?php p($l->t('These groups will still be able to receive shares, but not to initiate them.')); ?></em> + <em><?php p($l->t('These groups will still be able to receive shares, but not to initiate them.')); ?></em> </p> <p class="<?php if ($_['shareAPIEnabled'] === 'no') { diff --git a/apps/settings/tests/Settings/Admin/SharingTest.php b/apps/settings/tests/Settings/Admin/SharingTest.php index 041c4f1717a..2703eddcd5c 100644 --- a/apps/settings/tests/Settings/Admin/SharingTest.php +++ b/apps/settings/tests/Settings/Admin/SharingTest.php @@ -90,6 +90,8 @@ class SharingTest extends TestCase { ['core', 'shareapi_remote_expire_after_n_days', '7', '7'], ['core', 'shareapi_enforce_remote_expire_date', 'no', 'no'], ]); + $this->shareManager->method('shareWithGroupMembersOnly') + ->willReturn(false); $expected = new TemplateResponse( 'settings', @@ -121,6 +123,7 @@ class SharingTest extends TestCase { 'shareDefaultRemoteExpireDateSet' => 'no', 'shareRemoteExpireAfterNDays' => '7', 'shareRemoteEnforceExpireDate' => 'no', + 'allowLinksExcludeGroups' => '', ], '' ); @@ -156,6 +159,8 @@ class SharingTest extends TestCase { ['core', 'shareapi_remote_expire_after_n_days', '7', '7'], ['core', 'shareapi_enforce_remote_expire_date', 'no', 'no'], ]); + $this->shareManager->method('shareWithGroupMembersOnly') + ->willReturn(false); $expected = new TemplateResponse( 'settings', @@ -187,6 +192,7 @@ class SharingTest extends TestCase { 'shareDefaultRemoteExpireDateSet' => 'no', 'shareRemoteExpireAfterNDays' => '7', 'shareRemoteEnforceExpireDate' => 'no', + 'allowLinksExcludeGroups' => '', ], '' ); diff --git a/lib/private/Server.php b/lib/private/Server.php index f7eaf9f3591..9047a7c454d 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1234,7 +1234,8 @@ class Server extends ServerContainer implements IServerContainer { $c->get(IMailer::class), $c->get(IURLGenerator::class), $c->get('ThemingDefaults'), - $c->get(IEventDispatcher::class) + $c->get(IEventDispatcher::class), + $c->get(IUserSession::class) ); return $manager; diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 983653b7661..5cb51dd0ad5 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -57,6 +57,7 @@ use OCP\ILogger; use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; +use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Mail\IMailer; use OCP\Security\Events\ValidatePasswordPolicyEvent; @@ -104,8 +105,6 @@ class Manager implements IManager { private $sharingDisabledForUsersCache; /** @var EventDispatcherInterface */ private $legacyDispatcher; - /** @var LegacyHooks */ - private $legacyHooks; /** @var IMailer */ private $mailer; /** @var IURLGenerator */ @@ -114,44 +113,26 @@ class Manager implements IManager { private $defaults; /** @var IEventDispatcher */ private $dispatcher; + private $userSession; - - /** - * Manager constructor. - * - * @param ILogger $logger - * @param IConfig $config - * @param ISecureRandom $secureRandom - * @param IHasher $hasher - * @param IMountManager $mountManager - * @param IGroupManager $groupManager - * @param IL10N $l - * @param IFactory $l10nFactory - * @param IProviderFactory $factory - * @param IUserManager $userManager - * @param IRootFolder $rootFolder - * @param EventDispatcherInterface $eventDispatcher - * @param IMailer $mailer - * @param IURLGenerator $urlGenerator - * @param \OC_Defaults $defaults - */ public function __construct( - ILogger $logger, - IConfig $config, - ISecureRandom $secureRandom, - IHasher $hasher, - IMountManager $mountManager, - IGroupManager $groupManager, - IL10N $l, - IFactory $l10nFactory, - IProviderFactory $factory, - IUserManager $userManager, - IRootFolder $rootFolder, - EventDispatcherInterface $legacyDispatcher, - IMailer $mailer, - IURLGenerator $urlGenerator, - \OC_Defaults $defaults, - IEventDispatcher $dispatcher + ILogger $logger, + IConfig $config, + ISecureRandom $secureRandom, + IHasher $hasher, + IMountManager $mountManager, + IGroupManager $groupManager, + IL10N $l, + IFactory $l10nFactory, + IProviderFactory $factory, + IUserManager $userManager, + IRootFolder $rootFolder, + EventDispatcherInterface $legacyDispatcher, + IMailer $mailer, + IURLGenerator $urlGenerator, + \OC_Defaults $defaults, + IEventDispatcher $dispatcher, + IUserSession $userSession ) { $this->logger = $logger; $this->config = $config; @@ -166,11 +147,11 @@ class Manager implements IManager { $this->rootFolder = $rootFolder; $this->legacyDispatcher = $legacyDispatcher; $this->sharingDisabledForUsersCache = new CappedMemoryCache(); - $this->legacyHooks = new LegacyHooks($this->legacyDispatcher); $this->mailer = $mailer; $this->urlGenerator = $urlGenerator; $this->defaults = $defaults; $this->dispatcher = $dispatcher; + $this->userSession = $userSession; } /** @@ -274,7 +255,7 @@ class Manager implements IManager { // And it should be a file or a folder if (!($share->getNode() instanceof \OCP\Files\File) && - !($share->getNode() instanceof \OCP\Files\Folder)) { + !($share->getNode() instanceof \OCP\Files\Folder)) { throw new \InvalidArgumentException('Path should be either a file or a folder'); } @@ -422,13 +403,13 @@ class Manager implements IManager { } if ($fullId === null && $expirationDate === null && $defaultExpireDate) { $expirationDate = new \DateTime(); - $expirationDate->setTime(0,0,0); + $expirationDate->setTime(0, 0, 0); $days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays); if ($days > $defaultExpireDays) { $days = $defaultExpireDays; } - $expirationDate->add(new \DateInterval('P'.$days.'D')); + $expirationDate->add(new \DateInterval('P' . $days . 'D')); } // If we enforce the expiration date check that is does not exceed @@ -498,13 +479,13 @@ class Manager implements IManager { if ($fullId === null && $expirationDate === null && $this->shareApiLinkDefaultExpireDate()) { $expirationDate = new \DateTime(); - $expirationDate->setTime(0,0,0); + $expirationDate->setTime(0, 0, 0); $days = (int)$this->config->getAppValue('core', 'link_defaultExpDays', $this->shareApiLinkDefaultExpireDays()); if ($days > $this->shareApiLinkDefaultExpireDays()) { $days = $this->shareApiLinkDefaultExpireDays(); } - $expirationDate->add(new \DateInterval('P'.$days.'D')); + $expirationDate->add(new \DateInterval('P' . $days . 'D')); } // If we enforce the expiration date check that is does not exceed @@ -553,8 +534,8 @@ class Manager implements IManager { $sharedWith = $this->userManager->get($share->getSharedWith()); // Verify we can share with this user $groups = array_intersect( - $this->groupManager->getUserGroupIds($sharedBy), - $this->groupManager->getUserGroupIds($sharedWith) + $this->groupManager->getUserGroupIds($sharedBy), + $this->groupManager->getUserGroupIds($sharedWith) ); if (empty($groups)) { $message_t = $this->l->t('Sharing is only allowed with group members'); @@ -919,7 +900,7 @@ class Manager implements IManager { '%1$s via %2$s', [ $initiatorDisplayName, - $instanceName + $instanceName, ] ); $message->setFrom([\OCP\Util::getDefaultEmailAddress($instanceName) => $senderName]); @@ -1102,7 +1083,7 @@ class Manager implements IManager { * @since 9.0.0 */ public function acceptShare(IShare $share, string $recipientId): IShare { - [$providerId, ] = $this->splitFullId($share->getFullId()); + [$providerId,] = $this->splitFullId($share->getFullId()); $provider = $this->factory->getProvider($providerId); if (!method_exists($provider, 'acceptShare')) { @@ -1127,10 +1108,10 @@ class Manager implements IManager { */ private function updateSharePasswordIfNeeded(IShare $share, IShare $originalShare) { $passwordsAreDifferent = ($share->getPassword() !== $originalShare->getPassword()) && - (($share->getPassword() !== null && $originalShare->getPassword() === null) || - ($share->getPassword() === null && $originalShare->getPassword() !== null) || - ($share->getPassword() !== null && $originalShare->getPassword() !== null && - !$this->hasher->verify($share->getPassword(), $originalShare->getPassword()))); + (($share->getPassword() !== null && $originalShare->getPassword() === null) || + ($share->getPassword() === null && $originalShare->getPassword() !== null) || + ($share->getPassword() !== null && $originalShare->getPassword() !== null && + !$this->hasher->verify($share->getPassword(), $originalShare->getPassword()))); // Password updated. if ($passwordsAreDifferent) { @@ -1225,7 +1206,7 @@ class Manager implements IManager { * @param string $recipientId */ public function deleteFromSelf(IShare $share, $recipientId) { - [$providerId, ] = $this->splitFullId($share->getFullId()); + [$providerId,] = $this->splitFullId($share->getFullId()); $provider = $this->factory->getProvider($providerId); $provider->deleteFromSelf($share, $recipientId); @@ -1234,7 +1215,7 @@ class Manager implements IManager { } public function restoreShare(IShare $share, string $recipientId): IShare { - [$providerId, ] = $this->splitFullId($share->getFullId()); + [$providerId,] = $this->splitFullId($share->getFullId()); $provider = $this->factory->getProvider($providerId); return $provider->restore($share, $recipientId); @@ -1264,7 +1245,7 @@ class Manager implements IManager { } } - [$providerId, ] = $this->splitFullId($share->getFullId()); + [$providerId,] = $this->splitFullId($share->getFullId()); $provider = $this->factory->getProvider($providerId); return $provider->move($share, $recipientId); @@ -1291,8 +1272,8 @@ class Manager implements IManager { */ public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0) { if ($path !== null && - !($path instanceof \OCP\Files\File) && - !($path instanceof \OCP\Files\Folder)) { + !($path instanceof \OCP\Files\File) && + !($path instanceof \OCP\Files\Folder)) { throw new \InvalidArgumentException('invalid path'); } @@ -1533,8 +1514,8 @@ class Manager implements IManager { */ public function checkPassword(IShare $share, $password) { $passwordProtected = $share->getShareType() !== IShare::TYPE_LINK - || $share->getShareType() !== IShare::TYPE_EMAIL - || $share->getShareType() !== IShare::TYPE_CIRCLE; + || $share->getShareType() !== IShare::TYPE_EMAIL + || $share->getShareType() !== IShare::TYPE_CIRCLE; if (!$passwordProtected) { //TODO maybe exception? return false; @@ -1756,7 +1737,20 @@ class Manager implements IManager { * @return bool */ public function shareApiAllowLinks() { - return $this->config->getAppValue('core', 'shareapi_allow_links', 'yes') === 'yes'; + if ($this->config->getAppValue('core', 'shareapi_allow_links', 'yes') !== 'yes') { + return false; + } + + $user = $this->userSession->getUser(); + if ($user) { + $excludedGroups = json_decode($this->config->getAppValue('core', 'shareapi_allow_links_exclude_groups', '[]')); + if ($excludedGroups) { + $userGroups = $this->groupManager->getUserGroupIds($user); + return !(bool)array_intersect($excludedGroups, $userGroups); + } + } + + return true; } /** @@ -1780,6 +1774,7 @@ class Manager implements IManager { /** * Is default link expire date enforced *` + * * @return bool */ public function shareApiLinkDefaultExpireDateEnforced() { @@ -1790,6 +1785,7 @@ class Manager implements IManager { /** * Number of default link expire days + * * @return int */ public function shareApiLinkDefaultExpireDays() { @@ -1836,6 +1832,7 @@ class Manager implements IManager { /** * Number of default expire days + * * @return int */ public function shareApiInternalDefaultExpireDays(): int { @@ -1844,6 +1841,7 @@ class Manager implements IManager { /** * Number of default expire days for remote shares + * * @return int */ public function shareApiRemoteDefaultExpireDays(): int { @@ -1861,6 +1859,7 @@ class Manager implements IManager { /** * check if user can only share with group members + * * @return bool */ public function shareWithGroupMembersOnly() { @@ -1869,6 +1868,7 @@ class Manager implements IManager { /** * Check if users can share with groups + * * @return bool */ public function allowGroupSharing() { diff --git a/lib/private/legacy/OC_Util.php b/lib/private/legacy/OC_Util.php index 63eaf303759..c9e19221f95 100644 --- a/lib/private/legacy/OC_Util.php +++ b/lib/private/legacy/OC_Util.php @@ -72,6 +72,7 @@ use OCP\IGroupManager; use OCP\ILogger; use OCP\IUser; use OCP\IUserSession; +use OCP\Share\IManager; use Psr\Log\LoggerInterface; class OC_Util { @@ -336,8 +337,9 @@ class OC_Util { * @suppress PhanDeprecatedFunction */ public static function isPublicLinkPasswordRequired() { - $enforcePassword = \OC::$server->getConfig()->getAppValue('core', 'shareapi_enforce_links_password', 'no'); - return $enforcePassword === 'yes'; + /** @var IManager $shareManager */ + $shareManager = \OC::$server->get(IManager::class); + return $shareManager->shareApiLinkEnforcePassword(); } /** @@ -348,25 +350,10 @@ class OC_Util { * @return bool */ public static function isSharingDisabledForUser(IConfig $config, IGroupManager $groupManager, $user) { - if ($config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes') { - $groupsList = $config->getAppValue('core', 'shareapi_exclude_groups_list', ''); - $excludedGroups = json_decode($groupsList); - if (is_null($excludedGroups)) { - $excludedGroups = explode(',', $groupsList); - $newValue = json_encode($excludedGroups); - $config->setAppValue('core', 'shareapi_exclude_groups_list', $newValue); - } - $usersGroups = $groupManager->getUserGroupIds($user); - if (!empty($usersGroups)) { - $remainingGroups = array_diff($usersGroups, $excludedGroups); - // if the user is only in groups which are disabled for sharing then - // sharing is also disabled for the user - if (empty($remainingGroups)) { - return true; - } - } - } - return false; + /** @var IManager $shareManager */ + $shareManager = \OC::$server->get(IManager::class); + $userId = $user ? $user->getUID() : null; + return $shareManager->sharingDisabledForUser($userId); } /** @@ -376,14 +363,9 @@ class OC_Util { * @suppress PhanDeprecatedFunction */ public static function isDefaultExpireDateEnforced() { - $isDefaultExpireDateEnabled = \OC::$server->getConfig()->getAppValue('core', 'shareapi_default_expire_date', 'no'); - $enforceDefaultExpireDate = false; - if ($isDefaultExpireDateEnabled === 'yes') { - $value = \OC::$server->getConfig()->getAppValue('core', 'shareapi_enforce_expire_date', 'no'); - $enforceDefaultExpireDate = $value === 'yes'; - } - - return $enforceDefaultExpireDate; + /** @var IManager $shareManager */ + $shareManager = \OC::$server->get(IManager::class); + return $shareManager->shareApiLinkDefaultExpireDateEnforced(); } /** diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index 606e6429918..66b4de9c4e5 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -347,6 +347,54 @@ interface IManager { public function shareApiLinkDefaultExpireDays(); /** + * Is default internal expire date enabled + * + * @return bool + * @since 22.0.0 + */ + public function shareApiInternalDefaultExpireDate(): bool; + + /** + * Is default remote expire date enabled + * + * @return bool + * @since 22.0.0 + */ + public function shareApiRemoteDefaultExpireDate(): bool; + + /** + * Is default expire date enforced + * + * @return bool + * @since 22.0.0 + */ + public function shareApiInternalDefaultExpireDateEnforced(): bool; + + /** + * Is default expire date enforced for remote shares + * + * @return bool + * @since 22.0.0 + */ + public function shareApiRemoteDefaultExpireDateEnforced(): bool; + + /** + * Number of default expire days + * + * @return int + * @since 22.0.0 + */ + public function shareApiInternalDefaultExpireDays(): int; + + /** + * Number of default expire days for remote shares + * + * @return int + * @since 22.0.0 + */ + public function shareApiRemoteDefaultExpireDays(): int; + + /** * Allow public upload on link shares * * @return bool diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index d5094341eeb..117adc95186 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -45,6 +45,7 @@ use OCP\IServerContainer; use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; +use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Mail\IMailer; use OCP\Security\Events\ValidatePasswordPolicyEvent; @@ -104,6 +105,7 @@ class ManagerTest extends \Test\TestCase { protected $urlGenerator; /** @var \OC_Defaults|MockObject */ protected $defaults; + protected $userSession; protected function setUp(): void { $this->logger = $this->createMock(ILogger::class); @@ -119,6 +121,7 @@ class ManagerTest extends \Test\TestCase { $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->defaults = $this->createMock(\OC_Defaults::class); $this->dispatcher = $this->createMock(IEventDispatcher::class); + $this->userSession = $this->createMock(IUserSession::class); $this->l10nFactory = $this->createMock(IFactory::class); $this->l = $this->createMock(IL10N::class); @@ -149,7 +152,8 @@ class ManagerTest extends \Test\TestCase { $this->mailer, $this->urlGenerator, $this->defaults, - $this->dispatcher + $this->dispatcher, + $this->userSession ); $this->defaultProvider = $this->createMock(DefaultShareProvider::class); @@ -178,7 +182,8 @@ class ManagerTest extends \Test\TestCase { $this->mailer, $this->urlGenerator, $this->defaults, - $this->dispatcher + $this->dispatcher, + $this->userSession ]); } @@ -2690,7 +2695,8 @@ class ManagerTest extends \Test\TestCase { $this->mailer, $this->urlGenerator, $this->defaults, - $this->dispatcher + $this->dispatcher, + $this->userSession ); $share = $this->createMock(IShare::class); @@ -2734,7 +2740,8 @@ class ManagerTest extends \Test\TestCase { $this->mailer, $this->urlGenerator, $this->defaults, - $this->dispatcher + $this->dispatcher, + $this->userSession ); $share = $this->createMock(IShare::class); @@ -2785,7 +2792,8 @@ class ManagerTest extends \Test\TestCase { $this->mailer, $this->urlGenerator, $this->defaults, - $this->dispatcher + $this->dispatcher, + $this->userSession ); $share = $this->createMock(IShare::class); @@ -4123,7 +4131,8 @@ class ManagerTest extends \Test\TestCase { $this->mailer, $this->urlGenerator, $this->defaults, - $this->dispatcher + $this->dispatcher, + $this->userSession ); $this->assertSame($expected, $manager->shareProviderExists($shareType) @@ -4156,7 +4165,8 @@ class ManagerTest extends \Test\TestCase { $this->mailer, $this->urlGenerator, $this->defaults, - $this->dispatcher + $this->dispatcher, + $this->userSession ); $factory->setProvider($this->defaultProvider); @@ -4220,7 +4230,8 @@ class ManagerTest extends \Test\TestCase { $this->mailer, $this->urlGenerator, $this->defaults, - $this->dispatcher + $this->dispatcher, + $this->userSession ); $factory->setProvider($this->defaultProvider); @@ -4336,7 +4347,8 @@ class ManagerTest extends \Test\TestCase { $this->mailer, $this->urlGenerator, $this->defaults, - $this->dispatcher + $this->dispatcher, + $this->userSession ); $factory->setProvider($this->defaultProvider); @@ -4461,7 +4473,8 @@ class ManagerTest extends \Test\TestCase { $this->mailer, $this->urlGenerator, $this->defaults, - $this->dispatcher + $this->dispatcher, + $this->userSession ); $factory->setProvider($this->defaultProvider); diff --git a/tests/lib/UtilTest.php b/tests/lib/UtilTest.php index 6e25ce16e75..e21a5323b1b 100644 --- a/tests/lib/UtilTest.php +++ b/tests/lib/UtilTest.php @@ -10,8 +10,6 @@ namespace Test; use OC_Util; use OCP\App\IAppManager; -use OCP\IConfig; -use OCP\IUser; /** * Class UtilTest @@ -172,53 +170,6 @@ class UtilTest extends \Test\TestCase { } /** - * @dataProvider dataProviderForTestIsSharingDisabledForUser - * @param array $groups existing groups - * @param array $membership groups the user belong to - * @param array $excludedGroups groups which should be excluded from sharing - * @param bool $expected expected result - */ - public function testIsSharingDisabledForUser($groups, $membership, $excludedGroups, $expected) { - $config = $this->getMockBuilder(IConfig::class)->disableOriginalConstructor()->getMock(); - $groupManager = $this->getMockBuilder('OCP\IGroupManager')->disableOriginalConstructor()->getMock(); - $user = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); - - $config - ->expects($this->at(0)) - ->method('getAppValue') - ->with('core', 'shareapi_exclude_groups', 'no') - ->willReturn('yes'); - $config - ->expects($this->at(1)) - ->method('getAppValue') - ->with('core', 'shareapi_exclude_groups_list') - ->willReturn(json_encode($excludedGroups)); - - $groupManager - ->expects($this->at(0)) - ->method('getUserGroupIds') - ->with($user) - ->willReturn($membership); - - $result = \OC_Util::isSharingDisabledForUser($config, $groupManager, $user); - - $this->assertSame($expected, $result); - } - - public function dataProviderForTestIsSharingDisabledForUser() { - return [ - // existing groups, groups the user belong to, groups excluded from sharing, expected result - [['g1', 'g2', 'g3'], [], ['g1'], false], - [['g1', 'g2', 'g3'], [], [], false], - [['g1', 'g2', 'g3'], ['g2'], ['g1'], false], - [['g1', 'g2', 'g3'], ['g2'], [], false], - [['g1', 'g2', 'g3'], ['g1', 'g2'], ['g1'], false], - [['g1', 'g2', 'g3'], ['g1', 'g2'], ['g1', 'g2'], true], - [['g1', 'g2', 'g3'], ['g1', 'g2'], ['g1', 'g2', 'g3'], true], - ]; - } - - /** * Test default apps * * @dataProvider defaultAppsProvider |