Skip to content

Commit 31af870

Browse files
salmart-devsalmart-dev
authored
Merge pull request #55147 from nextcloud/fixPublicShares
Reflect public shares in `isPublic` flag and fix permission check
2 parents 5b1d928 + 631318f commit 31af870

File tree

5 files changed

+123
-83
lines changed

5 files changed

+123
-83
lines changed

‎apps/dav/appinfo/v1/publicwebdav.php‎

Lines changed: 48 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -68,47 +68,55 @@
6868
$linkCheckPlugin = new PublicLinkCheckPlugin();
6969
$filesDropPlugin = new FilesDropPlugin();
7070

71-
$server = $serverFactory->createServer(false, $baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($authBackend, $linkCheckPlugin, $filesDropPlugin) {
72-
$isAjax = in_array('XMLHttpRequest', explode(',', $_SERVER['HTTP_X_REQUESTED_WITH'] ?? ''));
73-
/** @var FederatedShareProvider $shareProvider */
74-
$federatedShareProvider = Server::get(FederatedShareProvider::class);
75-
if ($federatedShareProvider->isOutgoingServer2serverShareEnabled() === false && !$isAjax) {
76-
// this is what is thrown when trying to access a non-existing share
77-
throw new \Sabre\DAV\Exception\NotAuthenticated();
78-
}
79-
80-
$share = $authBackend->getShare();
81-
$owner = $share->getShareOwner();
82-
$isReadable = $share->getPermissions() & Constants::PERMISSION_READ;
83-
$fileId = $share->getNodeId();
84-
85-
// FIXME: should not add storage wrappers outside of preSetup, need to find a better way
86-
$previousLog = Filesystem::logWarningWhenAddingStorageWrapper(false);
87-
Filesystem::addStorageWrapper('sharePermissions', function ($mountPoint, $storage) use ($share) {
88-
return new PermissionsMask(['storage' => $storage, 'mask' => $share->getPermissions() | Constants::PERMISSION_SHARE]);
71+
$server = $serverFactory->createServer(
72+
true,
73+
$baseuri,
74+
$requestUri,
75+
$authPlugin,
76+
function (\Sabre\DAV\Server $server) use (
77+
$authBackend,
78+
$linkCheckPlugin,
79+
$filesDropPlugin
80+
) {
81+
$isAjax = in_array('XMLHttpRequest', explode(',', $_SERVER['HTTP_X_REQUESTED_WITH'] ?? ''));
82+
/** @var FederatedShareProvider $shareProvider */
83+
$federatedShareProvider = Server::get(FederatedShareProvider::class);
84+
if ($federatedShareProvider->isOutgoingServer2serverShareEnabled() === false && !$isAjax) {
85+
// this is what is thrown when trying to access a non-existing share
86+
throw new \Sabre\DAV\Exception\NotAuthenticated();
87+
}
88+
89+
$share = $authBackend->getShare();
90+
$owner = $share->getShareOwner();
91+
$isReadable = $share->getPermissions() & Constants::PERMISSION_READ;
92+
$fileId = $share->getNodeId();
93+
94+
// FIXME: should not add storage wrappers outside of preSetup, need to find a better way
95+
$previousLog = Filesystem::logWarningWhenAddingStorageWrapper(false);
96+
Filesystem::addStorageWrapper('sharePermissions', function ($mountPoint, $storage) use ($share) {
97+
return new PermissionsMask(['storage' => $storage, 'mask' => $share->getPermissions() | Constants::PERMISSION_SHARE]);
98+
});
99+
Filesystem::addStorageWrapper('shareOwner', function ($mountPoint, $storage) use ($share) {
100+
return new PublicOwnerWrapper(['storage' => $storage, 'owner' => $share->getShareOwner()]);
101+
});
102+
Filesystem::logWarningWhenAddingStorageWrapper($previousLog);
103+
104+
$rootFolder = Server::get(IRootFolder::class);
105+
$userFolder = $rootFolder->getUserFolder($owner);
106+
$node = $userFolder->getFirstNodeById($fileId);
107+
if (!$node) {
108+
throw new \Sabre\DAV\Exception\NotFound();
109+
}
110+
$linkCheckPlugin->setFileInfo($node);
111+
112+
// If not readable (files_drop) enable the filesdrop plugin
113+
if (!$isReadable) {
114+
$filesDropPlugin->enable();
115+
}
116+
$filesDropPlugin->setShare($share);
117+
118+
return new View($node->getPath());
89119
});
90-
Filesystem::addStorageWrapper('shareOwner', function ($mountPoint, $storage) use ($share) {
91-
return new PublicOwnerWrapper(['storage' => $storage, 'owner' => $share->getShareOwner()]);
92-
});
93-
Filesystem::logWarningWhenAddingStorageWrapper($previousLog);
94-
95-
$rootFolder = Server::get(IRootFolder::class);
96-
$userFolder = $rootFolder->getUserFolder($owner);
97-
$node = $userFolder->getFirstNodeById($fileId);
98-
if (!$node) {
99-
throw new \Sabre\DAV\Exception\NotFound();
100-
}
101-
$linkCheckPlugin->setFileInfo($node);
102-
103-
// If not readable (files_drop) enable the filesdrop plugin
104-
if (!$isReadable) {
105-
$filesDropPlugin->enable();
106-
}
107-
$filesDropPlugin->setShare($share);
108-
109-
$view = new View($node->getPath());
110-
return $view;
111-
});
112120

113121
$server->addPlugin($linkCheckPlugin);
114122
$server->addPlugin($filesDropPlugin);

‎apps/dav/lib/Connector/Sabre/Directory.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ public function getChild($name, $info = null, ?IRequest $request = null, ?IL10N
181181
// If we are, then only PUT and MKCOL are allowed (see plugin)
182182
// so we are safe to return the directory without a risk of
183183
// leaking files and folders structure.
184-
if ($storage instanceof PublicShareWrapper) {
184+
if ($storage->instanceOfStorage(PublicShareWrapper::class)) {
185185
$share = $storage->getShare();
186186
$allowDirectory = ($share->getPermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ;
187187
}

‎apps/dav/lib/Connector/Sabre/ServerFactory.php‎

Lines changed: 69 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
use OCP\ITagManager;
3838
use OCP\IUserManager;
3939
use OCP\IUserSession;
40+
use OCP\L10N\IFactory;
4041
use OCP\SabrePluginEvent;
4142
use OCP\SystemTag\ISystemTagManager;
4243
use OCP\SystemTag\ISystemTagObjectMapper;
@@ -70,15 +71,13 @@ public function createServer(
7071
Plugin $authPlugin,
7172
callable $viewCallBack,
7273
): Server {
74+
// /public.php/webdav/ shows the files in the share in the root itself
75+
// and not under /public.php/webdav/files/{token} so we should keep
76+
// compatibility for that.
77+
$needsSharesInRoot = $baseUri === '/public.php/webdav/';
78+
$useCollection = $isPublicShare && !$needsSharesInRoot;
7379
$debugEnabled = $this->config->getSystemValue('debug', false);
74-
// Fire up server
75-
if ($isPublicShare) {
76-
$rootCollection = new SimpleCollection('root');
77-
$tree = new CachingTree($rootCollection);
78-
} else {
79-
$rootCollection = null;
80-
$tree = new ObjectTree();
81-
}
80+
[$tree, $rootCollection] = $this->getTree($useCollection);
8281
$server = new Server($tree);
8382
// Set URL explicitly due to reverse-proxy situations
8483
$server->httpRequest->setUrl($requestUri);
@@ -127,8 +126,8 @@ public function createServer(
127126
}
128127

129128
// wait with registering these until auth is handled and the filesystem is setup
130-
$server->on('beforeMethod:*', function () use ($server, $tree,
131-
$viewCallBack, $isPublicShare, $rootCollection, $debugEnabled): void {
129+
$server->on('beforeMethod:*', function () use ($server,
130+
$tree, $viewCallBack, $isPublicShare, $rootCollection, $debugEnabled): void {
132131
// ensure the skeleton is copied
133132
$userFolder = \OC::$server->getUserFolder();
134133

@@ -147,36 +146,8 @@ public function createServer(
147146
$root = new File($view, $rootInfo);
148147
}
149148

150-
if ($isPublicShare) {
151-
$userPrincipalBackend = new Principal(
152-
\OCP\Server::get(IUserManager::class),
153-
\OCP\Server::get(IGroupManager::class),
154-
\OCP\Server::get(IAccountManager::class),
155-
\OCP\Server::get(\OCP\Share\IManager::class),
156-
\OCP\Server::get(IUserSession::class),
157-
\OCP\Server::get(IAppManager::class),
158-
\OCP\Server::get(ProxyMapper::class),
159-
\OCP\Server::get(KnownUserService::class),
160-
\OCP\Server::get(IConfig::class),
161-
\OC::$server->getL10NFactory(),
162-
);
163-
164-
// Mount the share collection at /public.php/dav/shares/<share token>
165-
$rootCollection->addChild(new RootCollection(
166-
$root,
167-
$userPrincipalBackend,
168-
'principals/shares',
169-
));
170-
171-
// Mount the upload collection at /public.php/dav/uploads/<share token>
172-
$rootCollection->addChild(new \OCA\DAV\Upload\RootCollection(
173-
$userPrincipalBackend,
174-
'principals/shares',
175-
\OCP\Server::get(CleanupService::class),
176-
\OCP\Server::get(IRootFolder::class),
177-
\OCP\Server::get(IUserSession::class),
178-
\OCP\Server::get(\OCP\Share\IManager::class),
179-
));
149+
if ($rootCollection !== null) {
150+
$this->initRootCollection($rootCollection, $root);
180151
} else {
181152
/** @var ObjectTree $tree */
182153
$tree->init($root, $view, $this->mountManager);
@@ -191,7 +162,7 @@ public function createServer(
191162
$this->userSession,
192163
\OCP\Server::get(IFilenameValidator::class),
193164
\OCP\Server::get(IAccountManager::class),
194-
false,
165+
$isPublicShare,
195166
!$debugEnabled
196167
)
197168
);
@@ -252,4 +223,61 @@ public function createServer(
252223
}, 30); // priority 30: after auth (10) and acl(20), before lock(50) and handling the request
253224
return $server;
254225
}
226+
227+
/**
228+
* Returns a Tree object and, if $useCollection is true, the collection used
229+
* as root.
230+
*
231+
* @param bool $useCollection Whether to use a collection or the legacy
232+
* ObjectTree, which doesn't use collections.
233+
* @return array{0: CachingTree|ObjectTree, 1: SimpleCollection|null}
234+
*/
235+
public function getTree(bool $useCollection): array {
236+
if ($useCollection) {
237+
$rootCollection = new SimpleCollection('root');
238+
$tree = new CachingTree($rootCollection);
239+
return [$tree, $rootCollection];
240+
}
241+
242+
return [new ObjectTree(), null];
243+
}
244+
245+
/**
246+
* Adds the user's principal backend to $rootCollection.
247+
*/
248+
private function initRootCollection(SimpleCollection $rootCollection, Directory|File $root): void {
249+
$userPrincipalBackend = new Principal(
250+
\OCP\Server::get(IUserManager::class),
251+
\OCP\Server::get(IGroupManager::class),
252+
\OCP\Server::get(IAccountManager::class),
253+
\OCP\Server::get(\OCP\Share\IManager::class),
254+
\OCP\Server::get(IUserSession::class),
255+
\OCP\Server::get(IAppManager::class),
256+
\OCP\Server::get(ProxyMapper::class),
257+
\OCP\Server::get(KnownUserService::class),
258+
\OCP\Server::get(IConfig::class),
259+
\OCP\Server::get(IFactory::class),
260+
);
261+
262+
// Mount the share collection at /public.php/dav/files/<share token>
263+
$rootCollection->addChild(
264+
new RootCollection(
265+
$root,
266+
$userPrincipalBackend,
267+
'principals/shares',
268+
)
269+
);
270+
271+
// Mount the upload collection at /public.php/dav/uploads/<share token>
272+
$rootCollection->addChild(
273+
new \OCA\DAV\Upload\RootCollection(
274+
$userPrincipalBackend,
275+
'principals/shares',
276+
\OCP\Server::get(CleanupService::class),
277+
\OCP\Server::get(IRootFolder::class),
278+
\OCP\Server::get(IUserSession::class),
279+
\OCP\Server::get(\OCP\Share\IManager::class),
280+
)
281+
);
282+
}
255283
}

‎apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use OCP\Files\ForbiddenException;
2222
use OCP\Files\InvalidPathException;
2323
use OCP\Files\Mount\IMountPoint;
24+
use OCP\Files\Storage\IStorage;
2425
use OCP\Files\StorageNotAvailableException;
2526
use PHPUnit\Framework\MockObject\MockObject;
2627
use Test\Traits\UserTrait;
@@ -61,12 +62,16 @@ class DirectoryTest extends \Test\TestCase {
6162

6263
private View&MockObject $view;
6364
private FileInfo&MockObject $info;
65+
private IStorage&MockObject $storage;
6466

6567
protected function setUp(): void {
6668
parent::setUp();
6769

6870
$this->view = $this->createMock(View::class);
6971
$this->info = $this->createMock(FileInfo::class);
72+
$this->storage = $this->createMock(IStorage::class);
73+
$this->info->method('getStorage')
74+
->willReturn($this->storage);
7075
$this->info->method('isReadable')
7176
->willReturn(true);
7277
$this->info->method('getType')

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,6 @@
736736
</file>
737737
<file src="apps/dav/lib/Connector/Sabre/ServerFactory.php">
738738
<DeprecatedMethod>
739-
<code><![CDATA[getL10NFactory]]></code>
740739
<code><![CDATA[getUserFolder]]></code>
741740
</DeprecatedMethod>
742741
<InternalMethod>

0 commit comments

Comments
 (0)