Skip to content

Commit

Permalink
Merge pull request #40232 from nextcloud/sharing-mask-wrapper-26
Browse files Browse the repository at this point in the history
[26] move share permission logic to storage wrapper
  • Loading branch information
blizzz authored Sep 5, 2023
2 parents f755b50 + 065f636 commit 1c29d87
Show file tree
Hide file tree
Showing 15 changed files with 168 additions and 127 deletions.
4 changes: 4 additions & 0 deletions apps/files_sharing/tests/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
namespace OCA\Files_Sharing\Tests;

use OC\Files\Cache\Scanner;
use OC\Files\Filesystem;
use OC\Files\SetupManager;
use OCA\Files_Sharing\Controller\ShareAPIController;
use OCP\App\IAppManager;
use OCP\AppFramework\OCS\OCSBadRequestException;
Expand Down Expand Up @@ -74,6 +76,8 @@ protected function setUp(): void {
\OC::$server->getConfig()->setAppValue('core', 'shareapi_exclude_groups', 'no');
\OC::$server->getConfig()->setAppValue('core', 'shareapi_expire_after_n_days', '7');

Filesystem::getLoader()->removeStorageWrapper('sharing_mask');

$this->folder = self::TEST_FOLDER_NAME;
$this->subfolder = '/subfolder_share_api_test';
$this->subsubfolder = '/subsubfolder_share_api_test';
Expand Down
4 changes: 3 additions & 1 deletion apps/files_sharing/tests/CapabilitiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

use OC\KnownUser\KnownUserService;
use OC\Share20\Manager;
use OC\Share20\ShareDisableChecker;
use OCA\Files_Sharing\Capabilities;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\IRootFolder;
Expand Down Expand Up @@ -96,7 +97,8 @@ private function getResults(array $map) {
$this->createMock(\OC_Defaults::class),
$this->createMock(IEventDispatcher::class),
$this->createMock(IUserSession::class),
$this->createMock(KnownUserService::class)
$this->createMock(KnownUserService::class),
$this->createMock(ShareDisableChecker::class)
);
$cap = new Capabilities($config, $shareManager);
$result = $this->getFilesSharingPart($cap->getCapabilities());
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1581,6 +1581,7 @@
'OC\\Share20\\PublicShareTemplateFactory' => $baseDir . '/lib/private/Share20/PublicShareTemplateFactory.php',
'OC\\Share20\\Share' => $baseDir . '/lib/private/Share20/Share.php',
'OC\\Share20\\ShareAttributes' => $baseDir . '/lib/private/Share20/ShareAttributes.php',
'OC\\Share20\\ShareDisableChecker' => $baseDir . '/lib/private/Share20/ShareDisableChecker.php',
'OC\\Share20\\ShareHelper' => $baseDir . '/lib/private/Share20/ShareHelper.php',
'OC\\Share20\\UserRemovedListener' => $baseDir . '/lib/private/Share20/UserRemovedListener.php',
'OC\\Share\\Constants' => $baseDir . '/lib/private/Share/Constants.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1614,6 +1614,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Share20\\PublicShareTemplateFactory' => __DIR__ . '/../../..' . '/lib/private/Share20/PublicShareTemplateFactory.php',
'OC\\Share20\\Share' => __DIR__ . '/../../..' . '/lib/private/Share20/Share.php',
'OC\\Share20\\ShareAttributes' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareAttributes.php',
'OC\\Share20\\ShareDisableChecker' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareDisableChecker.php',
'OC\\Share20\\ShareHelper' => __DIR__ . '/../../..' . '/lib/private/Share20/ShareHelper.php',
'OC\\Share20\\UserRemovedListener' => __DIR__ . '/../../..' . '/lib/private/Share20/UserRemovedListener.php',
'OC\\Share\\Constants' => __DIR__ . '/../../..' . '/lib/private/Share/Constants.php',
Expand Down
8 changes: 2 additions & 6 deletions lib/private/Files/FileInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ public function isEncrypted() {
}

/**
* Return the currently version used for the HMAC in the encryption app
* Return the current version used for the HMAC in the encryption app
*
* @return int
*/
Expand All @@ -247,11 +247,7 @@ public function getEncryptedVersion() {
* @return int
*/
public function getPermissions() {
$perms = (int) $this->data['permissions'];
if (\OCP\Util::isSharingDisabledForUser() || ($this->isShared() && !\OC\Share\Share::isResharingAllowed())) {
$perms = $perms & ~\OCP\Constants::PERMISSION_SHARE;
}
return $perms;
return (int) $this->data['permissions'];
}

/**
Expand Down
3 changes: 2 additions & 1 deletion lib/private/Files/ObjectStore/HomeObjectStoreStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
namespace OC\Files\ObjectStore;

use OC\User\User;
use OCP\IUser;

class HomeObjectStoreStorage extends ObjectStoreStorage implements \OCP\Files\IHomeStorage {
/**
Expand Down Expand Up @@ -61,7 +62,7 @@ public function getOwner($path) {
* @param string $path, optional
* @return \OC\User\User
*/
public function getUser($path = null) {
public function getUser($path = null): IUser {
return $this->user;
}
}
68 changes: 30 additions & 38 deletions lib/private/Files/SetupManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@
use OC\Files\Storage\Wrapper\PermissionsMask;
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;
use OCA\Files_Sharing\ISharedStorage;
use OCP\Constants;
use OCP\Diagnostics\IEventLogger;
use OCP\EventDispatcher\IEventDispatcher;
Expand Down Expand Up @@ -64,52 +67,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<string, string[]> */
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();
}
Expand Down Expand Up @@ -139,15 +123,23 @@ private function setupBuiltinWrappers() {
return $storage;
});

Filesystem::addStorageWrapper('enable_sharing', function ($mountPoint, IStorage $storage, IMountPoint $mount) {
if (!$mount->getOption('enable_sharing', true)) {
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) {
Expand Down
45 changes: 14 additions & 31 deletions lib/private/Files/SetupManagerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

namespace OC\Files;

use OC\Share20\ShareDisableChecker;
use OCP\Diagnostics\IEventLogger;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Config\IMountProviderCollection;
Expand All @@ -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;
}

Expand All @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion lib/private/Files/Storage/Home.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
namespace OC\Files\Storage;

use OC\Files\Cache\HomePropagator;
use OCP\IUser;

/**
* Specialized version of Local storage for home directory usage
Expand Down Expand Up @@ -94,7 +95,7 @@ public function getPropagator($storage = null) {
*
* @return \OC\User\User owner of this home storage
*/
public function getUser() {
public function getUser(): IUser {
return $this->user;
}

Expand Down
4 changes: 3 additions & 1 deletion lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@
use OC\Security\VerificationToken\VerificationToken;
use OC\Session\CryptoWrapper;
use OC\Share20\ProviderFactory;
use OC\Share20\ShareDisableChecker;
use OC\Share20\ShareHelper;
use OC\SystemTag\ManagerFactory as SystemTagManagerFactory;
use OC\Tagging\TagMapper;
Expand Down Expand Up @@ -1308,7 +1309,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;
Expand Down
41 changes: 5 additions & 36 deletions lib/private/Share20/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
*/
namespace OC\Share20;

use OCP\Cache\CappedMemoryCache;
use OC\Files\Mount\MoveableMount;
use OC\KnownUser\KnownUserService;
use OC\Share20\Exception\ProviderException;
Expand Down Expand Up @@ -103,8 +102,6 @@ class Manager implements IManager {
private $userManager;
/** @var IRootFolder */
private $rootFolder;
/** @var CappedMemoryCache */
private $sharingDisabledForUsersCache;
/** @var EventDispatcherInterface */
private $legacyDispatcher;
/** @var LegacyHooks */
Expand All @@ -121,6 +118,7 @@ class Manager implements IManager {
private $userSession;
/** @var KnownUserService */
private $knownUserService;
private ShareDisableChecker $shareDisableChecker;

public function __construct(
LoggerInterface $logger,
Expand All @@ -140,7 +138,8 @@ public function __construct(
\OC_Defaults $defaults,
IEventDispatcher $dispatcher,
IUserSession $userSession,
KnownUserService $knownUserService
KnownUserService $knownUserService,
ShareDisableChecker $shareDisableChecker
) {
$this->logger = $logger;
$this->config = $config;
Expand All @@ -154,7 +153,6 @@ public function __construct(
$this->userManager = $userManager;
$this->rootFolder = $rootFolder;
$this->legacyDispatcher = $legacyDispatcher;
$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($this->legacyDispatcher);
Expand All @@ -164,6 +162,7 @@ public function __construct(
$this->dispatcher = $dispatcher;
$this->userSession = $userSession;
$this->knownUserService = $knownUserService;
$this->shareDisableChecker = $shareDisableChecker;
}

/**
Expand Down Expand Up @@ -2016,37 +2015,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);
}

/**
Expand Down
Loading

0 comments on commit 1c29d87

Please sign in to comment.