Skip to content

Commit 3f9849d

Browse files
icewind1991icewind1991
authored
Merge pull request nextcloud#57295 from nextcloud/share-mount-validation-on-share
feat: perform share mount validation on share instead of on mount
2 parents dde6681 + 34fc215 commit 3f9849d

File tree

19 files changed

+463
-400
lines changed

19 files changed

+463
-400
lines changed

‎apps/files_sharing/composer/composer/autoload_classmap.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
'OCA\\Files_Sharing\\Listener\\LoadPublicFileRequestAuthListener' => $baseDir . '/../lib/Listener/LoadPublicFileRequestAuthListener.php',
7171
'OCA\\Files_Sharing\\Listener\\LoadSidebarListener' => $baseDir . '/../lib/Listener/LoadSidebarListener.php',
7272
'OCA\\Files_Sharing\\Listener\\ShareInteractionListener' => $baseDir . '/../lib/Listener/ShareInteractionListener.php',
73+
'OCA\\Files_Sharing\\Listener\\SharesUpdatedListener' => $baseDir . '/../lib/Listener/SharesUpdatedListener.php',
7374
'OCA\\Files_Sharing\\Listener\\UserAddedToGroupListener' => $baseDir . '/../lib/Listener/UserAddedToGroupListener.php',
7475
'OCA\\Files_Sharing\\Listener\\UserShareAcceptanceListener' => $baseDir . '/../lib/Listener/UserShareAcceptanceListener.php',
7576
'OCA\\Files_Sharing\\Middleware\\OCSShareAPIMiddleware' => $baseDir . '/../lib/Middleware/OCSShareAPIMiddleware.php',
@@ -96,6 +97,7 @@
9697
'OCA\\Files_Sharing\\Settings\\Personal' => $baseDir . '/../lib/Settings/Personal.php',
9798
'OCA\\Files_Sharing\\ShareBackend\\File' => $baseDir . '/../lib/ShareBackend/File.php',
9899
'OCA\\Files_Sharing\\ShareBackend\\Folder' => $baseDir . '/../lib/ShareBackend/Folder.php',
100+
'OCA\\Files_Sharing\\ShareTargetValidator' => $baseDir . '/../lib/ShareTargetValidator.php',
99101
'OCA\\Files_Sharing\\SharedMount' => $baseDir . '/../lib/SharedMount.php',
100102
'OCA\\Files_Sharing\\SharedStorage' => $baseDir . '/../lib/SharedStorage.php',
101103
'OCA\\Files_Sharing\\SharesReminderJob' => $baseDir . '/../lib/SharesReminderJob.php',

‎apps/files_sharing/composer/composer/autoload_static.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ class ComposerStaticInitFiles_Sharing
8585
'OCA\\Files_Sharing\\Listener\\LoadPublicFileRequestAuthListener' => __DIR__ . '/..' . '/../lib/Listener/LoadPublicFileRequestAuthListener.php',
8686
'OCA\\Files_Sharing\\Listener\\LoadSidebarListener' => __DIR__ . '/..' . '/../lib/Listener/LoadSidebarListener.php',
8787
'OCA\\Files_Sharing\\Listener\\ShareInteractionListener' => __DIR__ . '/..' . '/../lib/Listener/ShareInteractionListener.php',
88+
'OCA\\Files_Sharing\\Listener\\SharesUpdatedListener' => __DIR__ . '/..' . '/../lib/Listener/SharesUpdatedListener.php',
8889
'OCA\\Files_Sharing\\Listener\\UserAddedToGroupListener' => __DIR__ . '/..' . '/../lib/Listener/UserAddedToGroupListener.php',
8990
'OCA\\Files_Sharing\\Listener\\UserShareAcceptanceListener' => __DIR__ . '/..' . '/../lib/Listener/UserShareAcceptanceListener.php',
9091
'OCA\\Files_Sharing\\Middleware\\OCSShareAPIMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/OCSShareAPIMiddleware.php',
@@ -111,6 +112,7 @@ class ComposerStaticInitFiles_Sharing
111112
'OCA\\Files_Sharing\\Settings\\Personal' => __DIR__ . '/..' . '/../lib/Settings/Personal.php',
112113
'OCA\\Files_Sharing\\ShareBackend\\File' => __DIR__ . '/..' . '/../lib/ShareBackend/File.php',
113114
'OCA\\Files_Sharing\\ShareBackend\\Folder' => __DIR__ . '/..' . '/../lib/ShareBackend/Folder.php',
115+
'OCA\\Files_Sharing\\ShareTargetValidator' => __DIR__ . '/..' . '/../lib/ShareTargetValidator.php',
114116
'OCA\\Files_Sharing\\SharedMount' => __DIR__ . '/..' . '/../lib/SharedMount.php',
115117
'OCA\\Files_Sharing\\SharedStorage' => __DIR__ . '/..' . '/../lib/SharedStorage.php',
116118
'OCA\\Files_Sharing\\SharesReminderJob' => __DIR__ . '/..' . '/../lib/SharesReminderJob.php',

‎apps/files_sharing/lib/AppInfo/Application.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OCA\Files\Event\LoadSidebar;
1515
use OCA\Files_Sharing\Capabilities;
1616
use OCA\Files_Sharing\Config\ConfigLexicon;
17+
use OCA\Files_Sharing\Event\UserShareAccessUpdatedEvent;
1718
use OCA\Files_Sharing\External\Manager;
1819
use OCA\Files_Sharing\External\MountProvider as ExternalMountProvider;
1920
use OCA\Files_Sharing\Helper;
@@ -24,6 +25,7 @@
2425
use OCA\Files_Sharing\Listener\LoadPublicFileRequestAuthListener;
2526
use OCA\Files_Sharing\Listener\LoadSidebarListener;
2627
use OCA\Files_Sharing\Listener\ShareInteractionListener;
28+
use OCA\Files_Sharing\Listener\SharesUpdatedListener;
2729
use OCA\Files_Sharing\Listener\UserAddedToGroupListener;
2830
use OCA\Files_Sharing\Listener\UserShareAcceptanceListener;
2931
use OCA\Files_Sharing\Middleware\OCSShareAPIMiddleware;
@@ -46,11 +48,14 @@
4648
use OCP\Files\Events\BeforeDirectFileDownloadEvent;
4749
use OCP\Files\Events\BeforeZipCreatedEvent;
4850
use OCP\Files\Events\Node\BeforeNodeReadEvent;
51+
use OCP\Files\Events\Node\FilesystemTornDownEvent;
4952
use OCP\Group\Events\GroupChangedEvent;
5053
use OCP\Group\Events\GroupDeletedEvent;
5154
use OCP\Group\Events\UserAddedEvent;
55+
use OCP\Group\Events\UserRemovedEvent;
5256
use OCP\IDBConnection;
5357
use OCP\IGroup;
58+
use OCP\Share\Events\BeforeShareDeletedEvent;
5459
use OCP\Share\Events\ShareCreatedEvent;
5560
use OCP\User\Events\UserChangedEvent;
5661
use OCP\User\Events\UserDeletedEvent;
@@ -109,6 +114,14 @@ function () use ($c) {
109114
// File request auth
110115
$context->registerEventListener(BeforeTemplateRenderedEvent::class, LoadPublicFileRequestAuthListener::class);
111116

117+
// Update mounts
118+
$context->registerEventListener(ShareCreatedEvent::class, SharesUpdatedListener::class);
119+
$context->registerEventListener(BeforeShareDeletedEvent::class, SharesUpdatedListener::class);
120+
$context->registerEventListener(UserAddedEvent::class, SharesUpdatedListener::class);
121+
$context->registerEventListener(UserRemovedEvent::class, SharesUpdatedListener::class);
122+
$context->registerEventListener(UserShareAccessUpdatedEvent::class, SharesUpdatedListener::class);
123+
$context->registerEventListener(FilesystemTornDownEvent::class, SharesUpdatedListener::class);
124+
112125
$context->registerConfigLexicon(ConfigLexicon::class);
113126
}
114127

‎apps/files_sharing/lib/Listener/SharesUpdatedListener.php‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2025 Robin Appelman <robin@icewind.nl>
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Files_Sharing\Listener;
10+
11+
use OCA\Files_Sharing\Event\UserShareAccessUpdatedEvent;
12+
use OCA\Files_Sharing\MountProvider;
13+
use OCA\Files_Sharing\ShareTargetValidator;
14+
use OCP\Cache\CappedMemoryCache;
15+
use OCP\EventDispatcher\Event;
16+
use OCP\EventDispatcher\IEventListener;
17+
use OCP\Files\Config\ICachedMountInfo;
18+
use OCP\Files\Config\IUserMountCache;
19+
use OCP\Files\Events\Node\FilesystemTornDownEvent;
20+
use OCP\Group\Events\UserAddedEvent;
21+
use OCP\Group\Events\UserRemovedEvent;
22+
use OCP\IUser;
23+
use OCP\Share\Events\BeforeShareDeletedEvent;
24+
use OCP\Share\Events\ShareCreatedEvent;
25+
use OCP\Share\IManager;
26+
27+
/**
28+
* Listen to various events that can change what shares a user has access to
29+
*
30+
* @template-implements IEventListener<UserAddedEvent|UserRemovedEvent|ShareCreatedEvent|BeforeShareDeletedEvent|UserShareAccessUpdatedEvent|FilesystemTornDownEvent>
31+
*/
32+
class SharesUpdatedListener implements IEventListener {
33+
private CappedMemoryCache $updatedUsers;
34+
35+
public function __construct(
36+
private readonly IManager $shareManager,
37+
private readonly IUserMountCache $userMountCache,
38+
private readonly MountProvider $shareMountProvider,
39+
private readonly ShareTargetValidator $shareTargetValidator,
40+
) {
41+
$this->updatedUsers = new CappedMemoryCache();
42+
}
43+
public function handle(Event $event): void {
44+
if ($event instanceof FilesystemTornDownEvent) {
45+
$this->updatedUsers = new CappedMemoryCache();
46+
}
47+
if ($event instanceof UserShareAccessUpdatedEvent) {
48+
foreach ($event->getUsers() as $user) {
49+
$this->updateForUser($user);
50+
}
51+
}
52+
if ($event instanceof UserAddedEvent || $event instanceof UserRemovedEvent) {
53+
$this->updateForUser($event->getUser());
54+
}
55+
if ($event instanceof ShareCreatedEvent || $event instanceof BeforeShareDeletedEvent) {
56+
foreach ($this->shareManager->getUsersForShare($event->getShare()) as $user) {
57+
$this->updateForUser($user);
58+
}
59+
}
60+
}
61+
62+
private function updateForUser(IUser $user): void {
63+
if (isset($this->updatedUsers[$user->getUID()])) {
64+
return;
65+
}
66+
$this->updatedUsers[$user->getUID()] = true;
67+
68+
$cachedMounts = $this->userMountCache->getMountsForUser($user);
69+
$mountPoints = array_map(fn (ICachedMountInfo $mount) => $mount->getMountPoint(), $cachedMounts);
70+
$mountsByPath = array_combine($mountPoints, $cachedMounts);
71+
72+
$shares = $this->shareMountProvider->getSuperSharesForUser($user);
73+
74+
foreach ($shares as &$share) {
75+
[$parentShare, $groupedShares] = $share;
76+
$mountPoint = '/' . $user->getUID() . '/files/' . trim($parentShare->getTarget(), '/') . '/';
77+
$mountKey = $parentShare->getNodeId() . '::' . $mountPoint;
78+
if (!isset($cachedMounts[$mountKey])) {
79+
$this->shareTargetValidator->verifyMountPoint($user, $parentShare, $mountsByPath, $groupedShares);
80+
}
81+
}
82+
}
83+
}

‎apps/files_sharing/lib/MountProvider.php‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,14 @@ public function __construct(
5252
* @return IMountPoint[]
5353
*/
5454
public function getMountsForUser(IUser $user, IStorageFactory $loader) {
55+
return array_values($this->getMountsFromSuperShares($user, $this->getSuperSharesForUser($user), $loader));
56+
}
57+
58+
/**
59+
* @param IUser $user
60+
* @return list<array{IShare, array<IShare>}> Tuple of [superShare, groupedShares]
61+
*/
62+
public function getSuperSharesForUser(IUser $user): array {
5563
$userId = $user->getUID();
5664
$shares = $this->mergeIterables(
5765
$this->shareManager->getSharedWith($userId, IShare::TYPE_USER, null, -1),
@@ -62,16 +70,7 @@ public function getMountsForUser(IUser $user, IStorageFactory $loader) {
6270
);
6371

6472
$shares = $this->filterShares($shares, $userId);
65-
$superShares = $this->buildSuperShares($shares, $user);
66-
67-
return array_values(
68-
$this->getMountsFromSuperShares(
69-
$userId,
70-
$superShares,
71-
$loader,
72-
$user,
73-
),
74-
);
73+
return $this->buildSuperShares($shares, $user);
7574
}
7675

7776
/**
@@ -253,18 +252,18 @@ private function adjustTarget(
253252
}
254253
/**
255254
* @param string $userId
256-
* @param array $superShares
255+
* @param list<array{IShare, array<IShare>}> $superShares
257256
* @param IStorageFactory $loader
258257
* @param IUser $user
259258
* @return array IMountPoint indexed by mount point
260259
* @throws Exception
261260
*/
262-
private function getMountsFromSuperShares(
263-
string $userId,
261+
public function getMountsFromSuperShares(
262+
IUser $user,
264263
array $superShares,
265264
IStorageFactory $loader,
266-
IUser $user,
267265
): array {
266+
$userId = $user->getUID();
268267
$allMounts = $this->mountManager->getAll();
269268
$mounts = [];
270269
$view = new View('/' . $userId . '/files');
@@ -295,7 +294,6 @@ private function getMountsFromSuperShares(
295294
$shareId = (int)$parentShare->getId();
296295
$mount = new SharedMount(
297296
'\OCA\Files_Sharing\SharedStorage',
298-
$allMounts,
299297
[
300298
'user' => $userId,
301299
// parent share
@@ -306,11 +304,8 @@ private function getMountsFromSuperShares(
306304
'sharingDisabledForUser' => $sharingDisabledForUser
307305
],
308306
$loader,
309-
$view,
310-
$foldersExistCache,
311307
$this->eventDispatcher,
312308
$user,
313-
$shareId <= $maxValidatedShare,
314309
);
315310

316311
$newMaxValidatedShare = max($shareId, $newMaxValidatedShare);
@@ -395,7 +390,7 @@ public function getMountsForPath(
395390
$shares = $this->filterShares($shares, $userId);
396391
$superShares = $this->buildSuperShares($shares, $user);
397392

398-
return $this->getMountsFromSuperShares($userId, $superShares, $loader, $user);
393+
return $this->getMountsFromSuperShares($user, $superShares, $loader);
399394
}
400395

401396
/**

0 commit comments

Comments
 (0)