Skip to content

Commit f0e4d1a

Browse files
icewind1991icewind1991
committed
cleanup di for share permissions wrapper
Signed-off-by: Robin Appelman <[email protected]>
1 parent 4d2e520 commit f0e4d1a

8 files changed

Lines changed: 146 additions & 126 deletions

File tree

‎apps/files_sharing/tests/CapabilitiesTest.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
use OC\KnownUser\KnownUserService;
3232
use OC\Share20\Manager;
33+
use OC\Share20\ShareDisableChecker;
3334
use OCA\Files_Sharing\Capabilities;
3435
use OCP\EventDispatcher\IEventDispatcher;
3536
use OCP\Files\IRootFolder;
@@ -96,7 +97,8 @@ private function getResults(array $map) {
9697
$this->createMock(\OC_Defaults::class),
9798
$this->createMock(IEventDispatcher::class),
9899
$this->createMock(IUserSession::class),
99-
$this->createMock(KnownUserService::class)
100+
$this->createMock(KnownUserService::class),
101+
$this->createMock(ShareDisableChecker::class)
100102
);
101103
$cap = new Capabilities($config, $shareManager);
102104
$result = $this->getFilesSharingPart($cap->getCapabilities());

‎lib/private/Files/SetupManager.php‎

Lines changed: 28 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
use OC\Files\Storage\Wrapper\Quota;
3636
use OC\Lockdown\Filesystem\NullStorage;
3737
use OC\Share\Share;
38+
use OC\Share20\ShareDisableChecker;
3839
use OC_App;
3940
use OC_Hook;
4041
use OC_Util;
@@ -62,57 +63,37 @@
6263
use OCP\IUserSession;
6364
use OCP\Lockdown\ILockdownManager;
6465
use OCP\Share\Events\ShareCreatedEvent;
65-
use OCP\Share\IManager;
6666
use Psr\Log\LoggerInterface;
6767

6868
class SetupManager {
6969
private bool $rootSetup = false;
70-
private IEventLogger $eventLogger;
71-
private MountProviderCollection $mountProviderCollection;
72-
private IMountManager $mountManager;
73-
private IUserManager $userManager;
7470
// List of users for which at least one mount is setup
7571
private array $setupUsers = [];
7672
// List of users for which all mounts are setup
7773
private array $setupUsersComplete = [];
7874
/** @var array<string, string[]> */
7975
private array $setupUserMountProviders = [];
80-
private IEventDispatcher $eventDispatcher;
81-
private IUserMountCache $userMountCache;
82-
private ILockdownManager $lockdownManager;
83-
private IUserSession $userSession;
8476
private ICache $cache;
85-
private LoggerInterface $logger;
86-
private IConfig $config;
8777
private bool $listeningForProviders;
8878
private array $fullSetupRequired = [];
8979
private bool $setupBuiltinWrappersDone = false;
9080

9181
public function __construct(
92-
IEventLogger $eventLogger,
93-
MountProviderCollection $mountProviderCollection,
94-
IMountManager $mountManager,
95-
IUserManager $userManager,
96-
IEventDispatcher $eventDispatcher,
97-
IUserMountCache $userMountCache,
98-
ILockdownManager $lockdownManager,
99-
IUserSession $userSession,
82+
private IEventLogger $eventLogger,
83+
private MountProviderCollection $mountProviderCollection,
84+
private IMountManager $mountManager,
85+
private IUserManager $userManager,
86+
private IEventDispatcher $eventDispatcher,
87+
private IUserMountCache $userMountCache,
88+
private ILockdownManager $lockdownManager,
89+
private IUserSession $userSession,
10090
ICacheFactory $cacheFactory,
101-
LoggerInterface $logger,
102-
IConfig $config
91+
private LoggerInterface $logger,
92+
private IConfig $config,
93+
private ShareDisableChecker $shareDisableChecker,
10394
) {
104-
$this->eventLogger = $eventLogger;
105-
$this->mountProviderCollection = $mountProviderCollection;
106-
$this->mountManager = $mountManager;
107-
$this->userManager = $userManager;
108-
$this->eventDispatcher = $eventDispatcher;
109-
$this->userMountCache = $userMountCache;
110-
$this->lockdownManager = $lockdownManager;
111-
$this->logger = $logger;
112-
$this->userSession = $userSession;
11395
$this->cache = $cacheFactory->createDistributed('setupmanager::');
11496
$this->listeningForProviders = false;
115-
$this->config = $config;
11697

11798
$this->setupListeners();
11899
}
@@ -142,24 +123,23 @@ private function setupBuiltinWrappers() {
142123
return $storage;
143124
});
144125

145-
Filesystem::addStorageWrapper('sharing_mask', function ($mountPoint, IStorage $storage, IMountPoint $mount) {
146-
$reSharingEnabled = Share::isResharingAllowed();
147-
$sharingEnabledForMount = $mount->getOption('enable_sharing', true);
148-
/** @var IUserSession $userSession */
149-
$userSession = \OC::$server->get(IUserSession::class);
150-
$user = $userSession->getUser();
151-
/** @var IManager $shareManager */
152-
$shareManager = \OC::$server->get(IManager::class);
153-
$sharingEnabledForUser = $user ? !$shareManager->sharingDisabledForUser($user->getUID()) : true;
154-
$isShared = $storage->instanceOfStorage(ISharedStorage::class);
155-
if (!$sharingEnabledForMount || !$sharingEnabledForUser || (!$reSharingEnabled && $isShared)) {
156-
return new PermissionsMask([
157-
'storage' => $storage,
158-
'mask' => Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE,
159-
]);
126+
$reSharingEnabled = Share::isResharingAllowed();
127+
$user = $this->userSession->getUser();
128+
$sharingEnabledForUser = $user ? !$this->shareDisableChecker->sharingDisabledForUser($user->getUID()) : true;
129+
Filesystem::addStorageWrapper(
130+
'sharing_mask',
131+
function ($mountPoint, IStorage $storage, IMountPoint $mount) use ($reSharingEnabled, $sharingEnabledForUser) {
132+
$sharingEnabledForMount = $mount->getOption('enable_sharing', true);
133+
$isShared = $storage->instanceOfStorage(ISharedStorage::class);
134+
if (!$sharingEnabledForMount || !$sharingEnabledForUser || (!$reSharingEnabled && $isShared)) {
135+
return new PermissionsMask([
136+
'storage' => $storage,
137+
'mask' => Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE,
138+
]);
139+
}
140+
return $storage;
160141
}
161-
return $storage;
162-
});
142+
);
163143

164144
// install storage availability wrapper, before most other wrappers
165145
Filesystem::addStorageWrapper('oc_availability', function ($mountPoint, IStorage $storage) {

‎lib/private/Files/SetupManagerFactory.php‎

Lines changed: 14 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
namespace OC\Files;
2525

26+
use OC\Share20\ShareDisableChecker;
2627
use OCP\Diagnostics\IEventLogger;
2728
use OCP\EventDispatcher\IEventDispatcher;
2829
use OCP\Files\Config\IMountProviderCollection;
@@ -36,40 +37,21 @@
3637
use Psr\Log\LoggerInterface;
3738

3839
class SetupManagerFactory {
39-
private IEventLogger $eventLogger;
40-
private IMountProviderCollection $mountProviderCollection;
41-
private IUserManager $userManager;
42-
private IEventDispatcher $eventDispatcher;
43-
private IUserMountCache $userMountCache;
44-
private ILockdownManager $lockdownManager;
45-
private IUserSession $userSession;
4640
private ?SetupManager $setupManager;
47-
private ICacheFactory $cacheFactory;
48-
private LoggerInterface $logger;
49-
private IConfig $config;
5041

5142
public function __construct(
52-
IEventLogger $eventLogger,
53-
IMountProviderCollection $mountProviderCollection,
54-
IUserManager $userManager,
55-
IEventDispatcher $eventDispatcher,
56-
IUserMountCache $userMountCache,
57-
ILockdownManager $lockdownManager,
58-
IUserSession $userSession,
59-
ICacheFactory $cacheFactory,
60-
LoggerInterface $logger,
61-
IConfig $config
43+
private IEventLogger $eventLogger,
44+
private IMountProviderCollection $mountProviderCollection,
45+
private IUserManager $userManager,
46+
private IEventDispatcher $eventDispatcher,
47+
private IUserMountCache $userMountCache,
48+
private ILockdownManager $lockdownManager,
49+
private IUserSession $userSession,
50+
private ICacheFactory $cacheFactory,
51+
private LoggerInterface $logger,
52+
private IConfig $config,
53+
private ShareDisableChecker $shareDisableChecker,
6254
) {
63-
$this->eventLogger = $eventLogger;
64-
$this->mountProviderCollection = $mountProviderCollection;
65-
$this->userManager = $userManager;
66-
$this->eventDispatcher = $eventDispatcher;
67-
$this->userMountCache = $userMountCache;
68-
$this->lockdownManager = $lockdownManager;
69-
$this->userSession = $userSession;
70-
$this->cacheFactory = $cacheFactory;
71-
$this->logger = $logger;
72-
$this->config = $config;
7355
$this->setupManager = null;
7456
}
7557

@@ -86,7 +68,8 @@ public function create(IMountManager $mountManager): SetupManager {
8668
$this->userSession,
8769
$this->cacheFactory,
8870
$this->logger,
89-
$this->config
71+
$this->config,
72+
$this->shareDisableChecker,
9073
);
9174
}
9275
return $this->setupManager;

‎lib/private/Server.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@
147147
use OC\Security\VerificationToken\VerificationToken;
148148
use OC\Session\CryptoWrapper;
149149
use OC\Share20\ProviderFactory;
150+
use OC\Share20\ShareDisableChecker;
150151
use OC\Share20\ShareHelper;
151152
use OC\SpeechToText\SpeechToTextManager;
152153
use OC\SystemTag\ManagerFactory as SystemTagManagerFactory;
@@ -1305,7 +1306,8 @@ public function __construct($webRoot, \OC\Config $config) {
13051306
$c->get('ThemingDefaults'),
13061307
$c->get(IEventDispatcher::class),
13071308
$c->get(IUserSession::class),
1308-
$c->get(KnownUserService::class)
1309+
$c->get(KnownUserService::class),
1310+
$c->get(ShareDisableChecker::class)
13091311
);
13101312

13111313
return $manager;

‎lib/private/Share20/Manager.php‎

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
*/
4242
namespace OC\Share20;
4343

44-
use OCP\Cache\CappedMemoryCache;
4544
use OC\Files\Mount\MoveableMount;
4645
use OC\KnownUser\KnownUserService;
4746
use OC\Share20\Exception\ProviderException;
@@ -121,6 +120,7 @@ class Manager implements IManager {
121120
private $userSession;
122121
/** @var KnownUserService */
123122
private $knownUserService;
123+
private ShareDisableChecker $shareDisableChecker;
124124

125125
public function __construct(
126126
LoggerInterface $logger,
@@ -140,7 +140,8 @@ public function __construct(
140140
\OC_Defaults $defaults,
141141
IEventDispatcher $dispatcher,
142142
IUserSession $userSession,
143-
KnownUserService $knownUserService
143+
KnownUserService $knownUserService,
144+
ShareDisableChecker $shareDisableChecker
144145
) {
145146
$this->logger = $logger;
146147
$this->config = $config;
@@ -164,6 +165,7 @@ public function __construct(
164165
$this->dispatcher = $dispatcher;
165166
$this->userSession = $userSession;
166167
$this->knownUserService = $knownUserService;
168+
$this->shareDisableChecker = $shareDisableChecker;
167169
}
168170

169171
/**
@@ -2020,37 +2022,7 @@ public function currentUserCanEnumerateTargetUser(?IUser $currentUser, IUser $ta
20202022
* @return bool
20212023
*/
20222024
public function sharingDisabledForUser($userId) {
2023-
if ($userId === null) {
2024-
return false;
2025-
}
2026-
2027-
if (isset($this->sharingDisabledForUsersCache[$userId])) {
2028-
return $this->sharingDisabledForUsersCache[$userId];
2029-
}
2030-
2031-
if ($this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes') {
2032-
$groupsList = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', '');
2033-
$excludedGroups = json_decode($groupsList);
2034-
if (is_null($excludedGroups)) {
2035-
$excludedGroups = explode(',', $groupsList);
2036-
$newValue = json_encode($excludedGroups);
2037-
$this->config->setAppValue('core', 'shareapi_exclude_groups_list', $newValue);
2038-
}
2039-
$user = $this->userManager->get($userId);
2040-
$usersGroups = $this->groupManager->getUserGroupIds($user);
2041-
if (!empty($usersGroups)) {
2042-
$remainingGroups = array_diff($usersGroups, $excludedGroups);
2043-
// if the user is only in groups which are disabled for sharing then
2044-
// sharing is also disabled for the user
2045-
if (empty($remainingGroups)) {
2046-
$this->sharingDisabledForUsersCache[$userId] = true;
2047-
return true;
2048-
}
2049-
}
2050-
}
2051-
2052-
$this->sharingDisabledForUsersCache[$userId] = false;
2053-
return false;
2025+
return $this->shareDisableChecker->sharingDisabledForUser($userId);
20542026
}
20552027

20562028
/**

‎lib/private/Share20/ShareDisableChecker.php‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
namespace OC\Share20;
4+
5+
use OCP\Cache\CappedMemoryCache;
6+
use OCP\IConfig;
7+
use OCP\IGroupManager;
8+
use OCP\IUserManager;
9+
10+
/**
11+
* split of from the share manager to allow using it with minimal DI
12+
*/
13+
class ShareDisableChecker {
14+
private CappedMemoryCache $sharingDisabledForUsersCache;
15+
16+
public function __construct(
17+
private IConfig $config,
18+
private IUserManager $userManager,
19+
private IGroupManager $groupManager,
20+
) {
21+
$this->sharingDisabledForUsersCache = new CappedMemoryCache();
22+
}
23+
24+
25+
/**
26+
* @param ?string $userId
27+
* @return bool
28+
*/
29+
public function sharingDisabledForUser(?string $userId) {
30+
if ($userId === null) {
31+
return false;
32+
}
33+
34+
if (isset($this->sharingDisabledForUsersCache[$userId])) {
35+
return $this->sharingDisabledForUsersCache[$userId];
36+
}
37+
38+
if ($this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes') {
39+
$groupsList = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', '');
40+
$excludedGroups = json_decode($groupsList);
41+
if (is_null($excludedGroups)) {
42+
$excludedGroups = explode(',', $groupsList);
43+
$newValue = json_encode($excludedGroups);
44+
$this->config->setAppValue('core', 'shareapi_exclude_groups_list', $newValue);
45+
}
46+
$user = $this->userManager->get($userId);
47+
if (!$user) {
48+
return false;
49+
}
50+
$usersGroups = $this->groupManager->getUserGroupIds($user);
51+
if (!empty($usersGroups)) {
52+
$remainingGroups = array_diff($usersGroups, $excludedGroups);
53+
// if the user is only in groups which are disabled for sharing then
54+
// sharing is also disabled for the user
55+
if (empty($remainingGroups)) {
56+
$this->sharingDisabledForUsersCache[$userId] = true;
57+
return true;
58+
}
59+
}
60+
}
61+
62+
$this->sharingDisabledForUsersCache[$userId] = false;
63+
return false;
64+
}
65+
}

‎tests/lib/Files/ViewTest.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace Test\Files;
99

10+
use OC\Share20\ShareDisableChecker;
1011
use OCP\Cache\CappedMemoryCache;
1112
use OC\Files\Cache\Watcher;
1213
use OC\Files\Filesystem;
@@ -295,7 +296,7 @@ public function sharingDisabledPermissionProvider() {
295296
*/
296297
public function testRemoveSharePermissionWhenSharingDisabledForUser($excludeGroups, $excludeGroupsList, $expectedShareable) {
297298
// Reset sharing disabled for users cache
298-
self::invokePrivate(\OC::$server->getShareManager(), 'sharingDisabledForUsersCache', [new CappedMemoryCache()]);
299+
self::invokePrivate(\OC::$server->get(ShareDisableChecker::class), 'sharingDisabledForUsersCache', [new CappedMemoryCache()]);
299300

300301
$config = \OC::$server->getConfig();
301302
$oldExcludeGroupsFlag = $config->getAppValue('core', 'shareapi_exclude_groups', 'no');
@@ -320,7 +321,7 @@ public function testRemoveSharePermissionWhenSharingDisabledForUser($excludeGrou
320321
$config->setAppValue('core', 'shareapi_exclude_groups_list', $oldExcludeGroupsList);
321322

322323
// Reset sharing disabled for users cache
323-
self::invokePrivate(\OC::$server->getShareManager(), 'sharingDisabledForUsersCache', [new CappedMemoryCache()]);
324+
self::invokePrivate(\OC::$server->get(ShareDisableChecker::class), 'sharingDisabledForUsersCache', [new CappedMemoryCache()]);
324325
}
325326

326327
public function testCacheIncompleteFolder() {

0 commit comments

Comments
 (0)