Skip to content

Commit 59c26bf

Browse files
committed
fix(SecureView): hide disfunctional *download* files action
This is achieved by setting a specific DAV attribute. At the moment there is one handler in dav-apps FilesPlugin and it could overwrite the value with "false". We make sure not to downgrade here and prevent downgrade from dav (possible race condition). Signed-off-by: Arthur Schiwon <[email protected]>
1 parent 26c00e3 commit 59c26bf

File tree

3 files changed

+151
-0
lines changed

3 files changed

+151
-0
lines changed

lib/AppInfo/Application.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88

99
namespace OCA\Richdocuments\AppInfo;
1010

11+
use OCA\DAV\Events\SabrePluginAddEvent;
1112
use OCA\Files_Sharing\Event\ShareLinkAccessedEvent;
1213
use OCA\Richdocuments\AppConfig;
1314
use OCA\Richdocuments\Capabilities;
1415
use OCA\Richdocuments\Conversion\ConversionProvider;
1516
use OCA\Richdocuments\Db\WopiMapper;
1617
use OCA\Richdocuments\Listener\AddContentSecurityPolicyListener;
1718
use OCA\Richdocuments\Listener\AddFeaturePolicyListener;
19+
use OCA\Richdocuments\Listener\AddSabrePluginListener;
1820
use OCA\Richdocuments\Listener\BeforeFetchPreviewListener;
1921
use OCA\Richdocuments\Listener\BeforeGetTemplatesListener;
2022
use OCA\Richdocuments\Listener\BeforeTemplateRenderedListener;
@@ -49,6 +51,7 @@
4951
use OCP\AppFramework\Bootstrap\IBootstrap;
5052
use OCP\AppFramework\Bootstrap\IRegistrationContext;
5153
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
54+
use OCP\BeforeSabrePubliclyLoadedEvent;
5255
use OCP\Collaboration\Reference\RenderReferenceEvent;
5356
use OCP\Collaboration\Resources\LoadAdditionalScriptsEvent;
5457
use OCP\Files\Storage\IStorage;
@@ -86,6 +89,8 @@ public function register(IRegistrationContext $context): void {
8689
$context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class);
8790
$context->registerEventListener(BeforeGetTemplatesEvent::class, BeforeGetTemplatesListener::class);
8891
$context->registerEventListener(OverwritePublicSharePropertiesEvent::class, OverwritePublicSharePropertiesListener::class);
92+
$context->registerEventListener(SabrePluginAddEvent::class, AddSabrePluginListener::class);
93+
$context->registerEventListener(BeforeSabrePubliclyLoadedEvent::class, AddSabrePluginListener::class);
8994
$context->registerReferenceProvider(OfficeTargetReferenceProvider::class);
9095
$context->registerSensitiveMethods(WopiMapper::class, [
9196
'getPathForToken',

lib/DAV/SecureViewPlugin.php

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
namespace OCA\Richdocuments\DAV;
9+
10+
use OCA\DAV\Connector\Sabre\FilesPlugin;
11+
use OCA\DAV\Connector\Sabre\Node;
12+
use OCA\Richdocuments\AppConfig;
13+
use OCA\Richdocuments\Middleware\WOPIMiddleware;
14+
use OCA\Richdocuments\PermissionManager;
15+
use OCA\Richdocuments\Storage\SecureViewWrapper;
16+
use OCP\Files\ForbiddenException;
17+
use OCP\Files\NotFoundException;
18+
use OCP\Files\Storage\ISharedStorage;
19+
use OCP\Files\Storage\IStorage;
20+
use OCP\IAppConfig;
21+
use OCP\IUserSession;
22+
use Sabre\DAV\INode;
23+
use Sabre\DAV\PropFind;
24+
use Sabre\DAV\Server;
25+
use Sabre\DAV\ServerPlugin;
26+
27+
class SecureViewPlugin extends ServerPlugin {
28+
public function __construct(
29+
protected WOPIMiddleware $wopiMiddleware,
30+
protected IUserSession $userSession,
31+
protected PermissionManager $permissionManager,
32+
protected IAppConfig $appConfig,
33+
) {
34+
}
35+
36+
public function initialize(Server $server) {
37+
if ($this->appConfig->getValueString(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_enabled', 'no') === 'no') {
38+
return;
39+
}
40+
$server->on('propFind', $this->handleGetProperties(...));
41+
}
42+
43+
private function handleGetProperties(PropFind $propFind, INode $node): void {
44+
if (!$node instanceof Node) {
45+
return;
46+
}
47+
48+
$requestedProperties = $propFind->getRequestedProperties();
49+
if (!in_array(FilesPlugin::SHARE_HIDE_DOWNLOAD_PROPERTYNAME, $requestedProperties, true)) {
50+
return;
51+
}
52+
$currentValue = $propFind->get(FilesPlugin::SHARE_HIDE_DOWNLOAD_PROPERTYNAME);
53+
if ($currentValue === 'true') {
54+
// We won't unhide, hence can return early
55+
return;
56+
}
57+
58+
if (!$this->isDownloadable($node->getNode())) {
59+
// FIXME: coordinate with Files how a better solution looks like. Maybe by setting it only to 'true' by any provider? To avoid overwriting. Or throwing a dedicated event in just this case?
60+
$propFind->set(FilesPlugin::SHARE_HIDE_DOWNLOAD_PROPERTYNAME, 'true');
61+
// avoid potential race condition with FilesPlugin that may set it to "false"
62+
$propFind->handle(FilesPlugin::SHARE_HIDE_DOWNLOAD_PROPERTYNAME, 'true');
63+
}
64+
}
65+
66+
private function isDownloadable(\OCP\Files\Node $node): bool {
67+
try {
68+
$this->checkFileAccess($node->getInternalPath(), $node->getStorage());
69+
return true;
70+
} catch (ForbiddenException) {
71+
return false;
72+
}
73+
}
74+
75+
private function checkFileAccess(string $path, IStorage $storage): void {
76+
if ($this->wopiMiddleware->isWOPIRequest() || !$storage->instanceOfStorage(SecureViewWrapper::class)) {
77+
return;
78+
}
79+
80+
if ($this->shouldSecure($path, $storage)) {
81+
throw new ForbiddenException('Download blocked due the secure view policy', false);
82+
}
83+
}
84+
85+
// FIXME: remove duplication by moving into a SecureViewService
86+
private function shouldSecure(string $path, ?IStorage $sourceStorage = null): bool {
87+
if ($sourceStorage !== $this && $sourceStorage !== null) {
88+
$fp = $sourceStorage->fopen($path, 'r');
89+
fclose($fp);
90+
}
91+
92+
$storage = $sourceStorage ?? $this;
93+
$cacheEntry = $storage->getCache()->get($path);
94+
if (!$cacheEntry) {
95+
$parent = dirname($path);
96+
if ($parent === '.') {
97+
$parent = '';
98+
}
99+
$cacheEntry = $storage->getCache()->get($parent);
100+
if (!$cacheEntry) {
101+
throw new NotFoundException(sprintf('Could not find cache entry for path and parent of %s within storage %s ', $path, $storage->getId()));
102+
}
103+
}
104+
105+
$isSharedStorage = $storage->instanceOfStorage(ISharedStorage::class);
106+
107+
$share = $isSharedStorage ? $storage->getShare() : null;
108+
$userId = $this->userSession->getUser()?->getUID();
109+
110+
return $this->permissionManager->shouldWatermark($cacheEntry, $userId, $share, $storage->getOwner($path) ?: null);
111+
}
112+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
namespace OCA\Richdocuments\Listener;
9+
10+
use OCA\DAV\Events\SabrePluginAddEvent;
11+
use OCA\Richdocuments\DAV\SecureViewPlugin;
12+
use OCP\BeforeSabrePubliclyLoadedEvent;
13+
use OCP\EventDispatcher\Event;
14+
use OCP\EventDispatcher\IEventListener;
15+
use Psr\Container\ContainerInterface;
16+
17+
class AddSabrePluginListener implements IEventListener {
18+
19+
public function __construct(
20+
protected ContainerInterface $server,
21+
) {
22+
}
23+
24+
public function handle(Event $event): void {
25+
if (
26+
!$event instanceof SabrePluginAddEvent
27+
&& !$event instanceof BeforeSabrePubliclyLoadedEvent
28+
) {
29+
return;
30+
}
31+
$davServer = $event->getServer();
32+
$davServer->addPlugin($this->server->get(SecureViewPlugin::class));
33+
}
34+
}

0 commit comments

Comments
 (0)