From 1701426dc89b0dbd92ea81a749a89ed24bd31944 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 15 Aug 2023 18:21:04 +0200 Subject: [PATCH] cleanup di for share permissions wrapper Signed-off-by: Robin Appelman --- lib/private/Files/SetupManager.php | 75 ++++++++------------- lib/private/Files/SetupManagerFactory.php | 45 ++++--------- lib/private/Server.php | 4 +- lib/private/Share20/Manager.php | 40 ++--------- lib/private/Share20/ShareDisableChecker.php | 66 ++++++++++++++++++ 5 files changed, 116 insertions(+), 114 deletions(-) create mode 100644 lib/private/Share20/ShareDisableChecker.php diff --git a/lib/private/Files/SetupManager.php b/lib/private/Files/SetupManager.php index b1d609a9225dd..1a8aa4729370e 100644 --- a/lib/private/Files/SetupManager.php +++ b/lib/private/Files/SetupManager.php @@ -35,6 +35,7 @@ use OC\Files\Storage\Wrapper\Quota; use OC\Lockdown\Filesystem\NullStorage; use OC\Share\Share; +use OC\Share20\ShareDisableChecker; use OC_App; use OC_Hook; use OC_Util; @@ -67,52 +68,33 @@ class SetupManager { private bool $rootSetup = false; - private IEventLogger $eventLogger; - private MountProviderCollection $mountProviderCollection; - private IMountManager $mountManager; - private IUserManager $userManager; // List of users for which at least one mount is setup private array $setupUsers = []; // List of users for which all mounts are setup private array $setupUsersComplete = []; /** @var array */ private array $setupUserMountProviders = []; - private IEventDispatcher $eventDispatcher; - private IUserMountCache $userMountCache; - private ILockdownManager $lockdownManager; - private IUserSession $userSession; private ICache $cache; - private LoggerInterface $logger; - private IConfig $config; private bool $listeningForProviders; private array $fullSetupRequired = []; private bool $setupBuiltinWrappersDone = false; public function __construct( - IEventLogger $eventLogger, - MountProviderCollection $mountProviderCollection, - IMountManager $mountManager, - IUserManager $userManager, - IEventDispatcher $eventDispatcher, - IUserMountCache $userMountCache, - ILockdownManager $lockdownManager, - IUserSession $userSession, + private IEventLogger $eventLogger, + private MountProviderCollection $mountProviderCollection, + private IMountManager $mountManager, + private IUserManager $userManager, + private IEventDispatcher $eventDispatcher, + private IUserMountCache $userMountCache, + private ILockdownManager $lockdownManager, + private IUserSession $userSession, ICacheFactory $cacheFactory, - LoggerInterface $logger, - IConfig $config + private LoggerInterface $logger, + private IConfig $config, + private ShareDisableChecker $shareDisableChecker, ) { - $this->eventLogger = $eventLogger; - $this->mountProviderCollection = $mountProviderCollection; - $this->mountManager = $mountManager; - $this->userManager = $userManager; - $this->eventDispatcher = $eventDispatcher; - $this->userMountCache = $userMountCache; - $this->lockdownManager = $lockdownManager; - $this->logger = $logger; - $this->userSession = $userSession; $this->cache = $cacheFactory->createDistributed('setupmanager::'); $this->listeningForProviders = false; - $this->config = $config; $this->setupListeners(); } @@ -142,24 +124,23 @@ private function setupBuiltinWrappers() { return $storage; }); - Filesystem::addStorageWrapper('sharing_mask', function ($mountPoint, IStorage $storage, IMountPoint $mount) { - $reSharingEnabled = Share::isResharingAllowed(); - $sharingEnabledForMount = $mount->getOption('enable_sharing', true); - /** @var IUserSession $userSession */ - $userSession = \OC::$server->get(IUserSession::class); - $user = $userSession->getUser(); - /** @var IManager $shareManager */ - $shareManager = \OC::$server->get(IManager::class); - $sharingEnabledForUser = $user ? !$shareManager->sharingDisabledForUser($user->getUID()) : true; - $isShared = $storage->instanceOfStorage(ISharedStorage::class); - if (!$sharingEnabledForMount || !$sharingEnabledForUser || (!$reSharingEnabled && $isShared)) { - return new PermissionsMask([ - 'storage' => $storage, - 'mask' => Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE, - ]); + $reSharingEnabled = Share::isResharingAllowed(); + $user = $this->userSession->getUser(); + $sharingEnabledForUser = $user ? !$this->shareDisableChecker->sharingDisabledForUser($user->getUID()) : true; + Filesystem::addStorageWrapper( + 'sharing_mask', + function ($mountPoint, IStorage $storage, IMountPoint $mount) use ($reSharingEnabled, $sharingEnabledForUser) { + $sharingEnabledForMount = $mount->getOption('enable_sharing', true); + $isShared = $storage->instanceOfStorage(ISharedStorage::class); + if (!$sharingEnabledForMount || !$sharingEnabledForUser || (!$reSharingEnabled && $isShared)) { + return new PermissionsMask([ + 'storage' => $storage, + 'mask' => Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE, + ]); + } + return $storage; } - return $storage; - }); + ); // install storage availability wrapper, before most other wrappers Filesystem::addStorageWrapper('oc_availability', function ($mountPoint, IStorage $storage) { diff --git a/lib/private/Files/SetupManagerFactory.php b/lib/private/Files/SetupManagerFactory.php index 1d9efbd411fbe..8589cbdea42a4 100644 --- a/lib/private/Files/SetupManagerFactory.php +++ b/lib/private/Files/SetupManagerFactory.php @@ -23,6 +23,7 @@ namespace OC\Files; +use OC\Share20\ShareDisableChecker; use OCP\Diagnostics\IEventLogger; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IMountProviderCollection; @@ -36,40 +37,21 @@ use Psr\Log\LoggerInterface; class SetupManagerFactory { - private IEventLogger $eventLogger; - private IMountProviderCollection $mountProviderCollection; - private IUserManager $userManager; - private IEventDispatcher $eventDispatcher; - private IUserMountCache $userMountCache; - private ILockdownManager $lockdownManager; - private IUserSession $userSession; private ?SetupManager $setupManager; - private ICacheFactory $cacheFactory; - private LoggerInterface $logger; - private IConfig $config; public function __construct( - IEventLogger $eventLogger, - IMountProviderCollection $mountProviderCollection, - IUserManager $userManager, - IEventDispatcher $eventDispatcher, - IUserMountCache $userMountCache, - ILockdownManager $lockdownManager, - IUserSession $userSession, - ICacheFactory $cacheFactory, - LoggerInterface $logger, - IConfig $config + private IEventLogger $eventLogger, + private IMountProviderCollection $mountProviderCollection, + private IUserManager $userManager, + private IEventDispatcher $eventDispatcher, + private IUserMountCache $userMountCache, + private ILockdownManager $lockdownManager, + private IUserSession $userSession, + private ICacheFactory $cacheFactory, + private LoggerInterface $logger, + private IConfig $config, + private ShareDisableChecker $shareDisableChecker, ) { - $this->eventLogger = $eventLogger; - $this->mountProviderCollection = $mountProviderCollection; - $this->userManager = $userManager; - $this->eventDispatcher = $eventDispatcher; - $this->userMountCache = $userMountCache; - $this->lockdownManager = $lockdownManager; - $this->userSession = $userSession; - $this->cacheFactory = $cacheFactory; - $this->logger = $logger; - $this->config = $config; $this->setupManager = null; } @@ -86,7 +68,8 @@ public function create(IMountManager $mountManager): SetupManager { $this->userSession, $this->cacheFactory, $this->logger, - $this->config + $this->config, + $this->shareDisableChecker, ); } return $this->setupManager; diff --git a/lib/private/Server.php b/lib/private/Server.php index e9966e83caee9..f273d64944391 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -146,6 +146,7 @@ use OC\Security\VerificationToken\VerificationToken; use OC\Session\CryptoWrapper; use OC\Share20\ProviderFactory; +use OC\Share20\ShareDisableChecker; use OC\Share20\ShareHelper; use OC\SpeechToText\SpeechToTextManager; use OC\SystemTag\ManagerFactory as SystemTagManagerFactory; @@ -1237,7 +1238,8 @@ public function __construct($webRoot, \OC\Config $config) { $c->get('ThemingDefaults'), $c->get(IEventDispatcher::class), $c->get(IUserSession::class), - $c->get(KnownUserService::class) + $c->get(KnownUserService::class), + $c->get(ShareDisableChecker::class) ); return $manager; diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 9360046bc248e..748d3059e4cba 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -106,8 +106,6 @@ class Manager implements IManager { private $userManager; /** @var IRootFolder */ private $rootFolder; - /** @var CappedMemoryCache */ - private $sharingDisabledForUsersCache; /** @var LegacyHooks */ private $legacyHooks; /** @var IMailer */ @@ -122,6 +120,7 @@ class Manager implements IManager { private $userSession; /** @var KnownUserService */ private $knownUserService; + private ShareDisableChecker $shareDisableChecker; public function __construct( LoggerInterface $logger, @@ -140,7 +139,8 @@ public function __construct( \OC_Defaults $defaults, IEventDispatcher $dispatcher, IUserSession $userSession, - KnownUserService $knownUserService + KnownUserService $knownUserService, + ShareDisableChecker $shareDisableChecker ) { $this->logger = $logger; $this->config = $config; @@ -153,7 +153,6 @@ public function __construct( $this->factory = $factory; $this->userManager = $userManager; $this->rootFolder = $rootFolder; - $this->sharingDisabledForUsersCache = new CappedMemoryCache(); // The constructor of LegacyHooks registers the listeners of share events // do not remove if those are not properly migrated $this->legacyHooks = new LegacyHooks($dispatcher); @@ -163,6 +162,7 @@ public function __construct( $this->dispatcher = $dispatcher; $this->userSession = $userSession; $this->knownUserService = $knownUserService; + $this->shareDisableChecker = $shareDisableChecker; } /** @@ -2011,37 +2011,7 @@ public function currentUserCanEnumerateTargetUser(?IUser $currentUser, IUser $ta * @return bool */ public function sharingDisabledForUser($userId) { - if ($userId === null) { - return false; - } - - if (isset($this->sharingDisabledForUsersCache[$userId])) { - return $this->sharingDisabledForUsersCache[$userId]; - } - - if ($this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes') { - $groupsList = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', ''); - $excludedGroups = json_decode($groupsList); - if (is_null($excludedGroups)) { - $excludedGroups = explode(',', $groupsList); - $newValue = json_encode($excludedGroups); - $this->config->setAppValue('core', 'shareapi_exclude_groups_list', $newValue); - } - $user = $this->userManager->get($userId); - $usersGroups = $this->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)) { - $this->sharingDisabledForUsersCache[$userId] = true; - return true; - } - } - } - - $this->sharingDisabledForUsersCache[$userId] = false; - return false; + return $this->shareDisableChecker->sharingDisabledForUser($userId); } /** diff --git a/lib/private/Share20/ShareDisableChecker.php b/lib/private/Share20/ShareDisableChecker.php new file mode 100644 index 0000000000000..25978fc33095f --- /dev/null +++ b/lib/private/Share20/ShareDisableChecker.php @@ -0,0 +1,66 @@ +sharingDisabledForUsersCache = new CappedMemoryCache(); + } + + + /** + * Copied from \OC_Util::isSharingDisabledForUser + * + * TODO: Deprecate function from OC_Util + * + * @param string $userId + * @return bool + */ + public function sharingDisabledForUser($userId) { + if ($userId === null) { + return false; + } + + if (isset($this->sharingDisabledForUsersCache[$userId])) { + return $this->sharingDisabledForUsersCache[$userId]; + } + + if ($this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes') { + $groupsList = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', ''); + $excludedGroups = json_decode($groupsList); + if (is_null($excludedGroups)) { + $excludedGroups = explode(',', $groupsList); + $newValue = json_encode($excludedGroups); + $this->config->setAppValue('core', 'shareapi_exclude_groups_list', $newValue); + } + $user = $this->userManager->get($userId); + $usersGroups = $this->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)) { + $this->sharingDisabledForUsersCache[$userId] = true; + return true; + } + } + } + + $this->sharingDisabledForUsersCache[$userId] = false; + return false; + } +}