Skip to content

Commit 09e7457

Browse files
juliusknorrjuliusknorr
committed
feat: Add storage wrappert to block download on secure view
Signed-off-by: Julius Knorr <jus@bitgrid.net>
1 parent 53e4991 commit 09e7457

9 files changed

Lines changed: 229 additions & 13 deletions

‎lib/AppConfig.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,13 @@ public function useSecureViewAdditionalMimes(): bool {
204204
return $this->config->getAppValue(Application::APPNAME, self::USE_SECURE_VIEW_ADDITIONAL_MIMES, 'no') === 'yes';
205205
}
206206

207+
public function getMimeTypes(): array {
208+
return array_merge([
209+
Capabilities::MIMETYPES,
210+
Capabilities::MIMETYPES_MSOFFICE,
211+
]);
212+
}
213+
207214
public function getDomainList(): array {
208215
$urls = array_merge(
209216
[ $this->domainOnly($this->getCollaboraUrlPublic()) ],

‎lib/AppInfo/Application.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use OCA\Richdocuments\Listener\FileCreatedFromTemplateListener;
2121
use OCA\Richdocuments\Listener\LoadAdditionalListener;
2222
use OCA\Richdocuments\Listener\LoadViewerListener;
23+
use OCA\Richdocuments\Listener\OverwritePublicSharePropertiesListener;
2324
use OCA\Richdocuments\Listener\ReferenceListener;
2425
use OCA\Richdocuments\Listener\RegisterTemplateFileCreatorListener;
2526
use OCA\Richdocuments\Listener\ShareLinkListener;
@@ -32,13 +33,15 @@
3233
use OCA\Richdocuments\Preview\OpenDocument;
3334
use OCA\Richdocuments\Preview\Pdf;
3435
use OCA\Richdocuments\Reference\OfficeTargetReferenceProvider;
36+
use OCA\Richdocuments\Storage\SecureViewWrapper;
3537
use OCA\Richdocuments\TaskProcessing\SlideDeckGenerationProvider;
3638
use OCA\Richdocuments\TaskProcessing\SlideDeckGenerationTaskType;
3739
use OCA\Richdocuments\TaskProcessing\TextToDocumentProvider;
3840
use OCA\Richdocuments\TaskProcessing\TextToDocumentTaskType;
3941
use OCA\Richdocuments\TaskProcessing\TextToSpreadsheetProvider;
4042
use OCA\Richdocuments\TaskProcessing\TextToSpreadsheetTaskType;
4143
use OCA\Richdocuments\Template\CollaboraTemplateProvider;
44+
use OCA\Talk\Events\OverwritePublicSharePropertiesEvent;
4245
use OCA\Viewer\Event\LoadViewer;
4346
use OCP\AppFramework\App;
4447
use OCP\AppFramework\Bootstrap\IBootContext;
@@ -47,6 +50,7 @@
4750
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
4851
use OCP\Collaboration\Reference\RenderReferenceEvent;
4952
use OCP\Collaboration\Resources\LoadAdditionalScriptsEvent;
53+
use OCP\Files\Storage\IStorage;
5054
use OCP\Files\Template\BeforeGetTemplatesEvent;
5155
use OCP\Files\Template\FileCreatedFromTemplateEvent;
5256
use OCP\Files\Template\RegisterTemplateCreatorEvent;
@@ -62,6 +66,8 @@ public function __construct(array $urlParams = []) {
6266
}
6367

6468
public function register(IRegistrationContext $context): void {
69+
\OCP\Util::connectHook('OC_Filesystem', 'preSetup', $this, 'addStorageWrapper');
70+
6571
$context->registerTemplateProvider(CollaboraTemplateProvider::class);
6672
$context->registerCapability(Capabilities::class);
6773
$context->registerMiddleWare(WOPIMiddleware::class);
@@ -76,6 +82,7 @@ public function register(IRegistrationContext $context): void {
7682
$context->registerEventListener(RenderReferenceEvent::class, ReferenceListener::class);
7783
$context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class);
7884
$context->registerEventListener(BeforeGetTemplatesEvent::class, BeforeGetTemplatesListener::class);
85+
$context->registerEventListener(OverwritePublicSharePropertiesEvent::class, OverwritePublicSharePropertiesListener::class);
7986
$context->registerReferenceProvider(OfficeTargetReferenceProvider::class);
8087
$context->registerSensitiveMethods(WopiMapper::class, [
8188
'getPathForToken',
@@ -101,4 +108,30 @@ public function register(IRegistrationContext $context): void {
101108

102109
public function boot(IBootContext $context): void {
103110
}
111+
112+
/**
113+
* @internal
114+
*/
115+
public function addStorageWrapper(): void {
116+
// FIXME: We can skip this if secure view is not enabled at all
117+
// Needs to be added as the first layer
118+
\OC\Files\Filesystem::addStorageWrapper('richdocuments', [$this, 'addStorageWrapperCallback'], -10);
119+
}
120+
121+
/**
122+
* @param $mountPoint
123+
* @param IStorage $storage
124+
* @return SecureViewWrapper|IStorage
125+
*@internal
126+
*/
127+
public function addStorageWrapperCallback($mountPoint, IStorage $storage) {
128+
if (!\OC::$CLI && $mountPoint !== '/') {
129+
return new SecureViewWrapper([
130+
'storage' => $storage,
131+
'mountPoint' => $mountPoint,
132+
]);
133+
}
134+
135+
return $storage;
136+
}
104137
}

‎lib/Listener/OverwritePublicSharePropertiesListener.php‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
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\Richdocuments\PermissionManager;
11+
use OCA\Talk\Events\OverwritePublicSharePropertiesEvent;
12+
use OCP\EventDispatcher\Event;
13+
use OCP\EventDispatcher\IEventListener;
14+
use OCP\Files\NotFoundException;
15+
16+
/** @template-implements IEventListener<OverwritePublicSharePropertiesEvent|Event> */
17+
class OverwritePublicSharePropertiesListener implements IEventListener {
18+
public function __construct(
19+
private PermissionManager $permissionManager,
20+
private ?string $userId,
21+
) {
22+
}
23+
24+
public function handle(Event $event): void {
25+
if (!$event instanceof OverwritePublicSharePropertiesEvent) {
26+
return;
27+
}
28+
29+
$share = $event->getShare();
30+
try {
31+
$node = $share->getNode();
32+
} catch (NotFoundException) {
33+
return;
34+
}
35+
36+
if ($this->permissionManager->shouldWatermark($node, $this->userId, $share)) {
37+
$share->setHideDownload(true);
38+
}
39+
}
40+
}

‎lib/Middleware/WOPIMiddleware.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public function __construct(
3333
private IRequest $request,
3434
private WopiMapper $wopiMapper,
3535
private LoggerInterface $logger,
36+
private bool $isWOPIRequest = false,
3637
) {
3738
}
3839

@@ -78,6 +79,8 @@ public function beforeController($controller, $methodName) {
7879
$this->logger->error('Failed to validate WOPI access', [ 'exception' => $e ]);
7980
throw new NotPermittedException();
8081
}
82+
83+
$this->isWOPIRequest = true;
8184
}
8285

8386
public function afterException($controller, $methodName, \Exception $exception): Response {
@@ -110,4 +113,8 @@ public function isWOPIAllowed(): bool {
110113
$this->logger->warning('WOPI request denied from ' . $userIp . ' as it does not match the configured ranges: ' . implode(', ', $allowedRanges));
111114
return false;
112115
}
116+
117+
public function isWOPIRequest(): bool {
118+
return $this->isWOPIRequest;
119+
}
113120
}

‎lib/PermissionManager.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ public function shouldWatermark(Node $node, ?string $userId = null, ?IShare $sha
117117
return false;
118118
}
119119

120+
if (!in_array($node->getMimetype(), $this->appConfig->getMimeTypes(), true)) {
121+
return false;
122+
}
123+
120124
$fileId = $node->getId();
121125

122126
$isUpdatable = $node->isUpdateable() && (!$share || $share->getPermissions() & Constants::PERMISSION_UPDATE);
@@ -163,7 +167,7 @@ public function shouldWatermark(Node $node, ?string $userId = null, ?IShare $sha
163167
}
164168

165169
if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_shareTalkPublic', 'no') === 'yes') {
166-
if ($userId === null && $share->getShareType() === IShare::TYPE_ROOM) {
170+
if ($userId === null && $share?->getShareType() === IShare::TYPE_ROOM) {
167171
return true;
168172
}
169173
}

‎lib/Storage/SecureViewWrapper.php‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
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+
9+
namespace OCA\Richdocuments\Storage;
10+
11+
use OC\Files\Storage\Wrapper\Wrapper;
12+
use OCA\Richdocuments\Middleware\WOPIMiddleware;
13+
use OCA\Richdocuments\PermissionManager;
14+
use OCP\Files\ForbiddenException;
15+
use OCP\Files\IRootFolder;
16+
use OCP\Files\Storage\ISharedStorage;
17+
use OCP\IUserSession;
18+
use OCP\Server;
19+
20+
class SecureViewWrapper extends Wrapper {
21+
private PermissionManager $permissionManager;
22+
private WOPIMiddleware $wopiMiddleware;
23+
private IRootFolder $rootFolder;
24+
private IUserSession $userSession;
25+
26+
private string $mountPoint;
27+
28+
public function __construct(array $parameters) {
29+
parent::__construct($parameters);
30+
31+
$this->permissionManager = Server::get(PermissionManager::class);
32+
$this->wopiMiddleware = Server::get(WOPIMiddleware::class);
33+
$this->rootFolder = Server::get(IRootFolder::class);
34+
$this->userSession = Server::get(IUserSession::class);
35+
36+
$this->mountPoint = $parameters['mountPoint'];
37+
}
38+
39+
public function fopen($path, $mode) {
40+
$this->checkFileAccess($path);
41+
42+
return $this->storage->fopen($path, $mode);
43+
}
44+
45+
public function checkFileAccess($path) {
46+
$isWopiRequest = $this->wopiMiddleware->isWOPIRequest();
47+
48+
$isSharedStorage = $this->instanceOfStorage(ISharedStorage::class);
49+
$node = $this->rootFolder->get($this->mountPoint)->get($path);
50+
$share = $isSharedStorage && method_exists($this, 'getShare') ? $this->getShare() : null;
51+
$userId = $this->userSession->getUser()?->getUID();
52+
53+
if ($this->permissionManager->shouldWatermark($node, $userId, $share) && !$isWopiRequest) {
54+
throw new ForbiddenException('Download blocked due the secure view policy', false);
55+
}
56+
}
57+
}

‎tests/lib/PermissionManagerTest.php‎

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace Tests\Richdocuments;
77

88
use OCA\Richdocuments\AppConfig;
9+
use OCA\Richdocuments\Capabilities;
910
use OCA\Richdocuments\PermissionManager;
1011
use OCP\Files\Node;
1112
use OCP\IConfig;
@@ -136,24 +137,25 @@ public static function dataWatermarkTagIds(): array {
136137

137138
/** @dataProvider dataWatermarkTagIds */
138139
public function testShouldWatermarkOptionLinkTags(array $objectTagIds, array $watermarkTagIds): void {
139-
$node = $this->createMock(Node::class);
140-
$share = $this->createMock(IShare::class);
140+
$node = $this->createNodeMock();
141+
$share = $this->createShareMock(IShare::TYPE_LINK);
141142
$userId = 'testUserId';
142143

144+
$this->appConfig->expects($this->any())
145+
->method('getMimeTypes')
146+
->willReturn(Capabilities::MIMETYPES);
147+
143148
$this->config
144-
->expects($this->exactly(5))
145149
->method('getAppValue')
146150
->willReturnMap([
147151
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_enabled', 'no', 'yes'],
148152
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkAll', 'no', 'no'],
149153
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkRead', 'no', 'no'],
150154
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkSecure', 'no', 'no'],
151155
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkTags', 'no', 'yes'],
156+
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_shareTalkPublic', 'no', 'no'],
152157
]);
153158

154-
$node->expects($this->once())->method('getId')->willReturn('testFileId');
155-
$node->expects($this->once())->method('isUpdateable')->willReturn(false);
156-
157159
$share->expects($this->once())->method('getHideDownload')->willReturn(true);
158160
$share->expects($this->once())->method('getAttributes')->willReturn(null);
159161
$share->expects($this->once())->method('getShareType')->willReturn(IShare::TYPE_LINK);
@@ -167,12 +169,14 @@ public function testShouldWatermarkOptionLinkTags(array $objectTagIds, array $wa
167169

168170
/** @dataProvider dataWatermarkTagIds */
169171
public function testShouldWatermarkOptionAllTags(array $objectTagIds, array $watermarkTagIds): void {
170-
$node = $this->createMock(Node::class);
171-
$share = $this->createMock(IShare::class);
172+
$node = $this->createNodeMock();
172173
$userId = 'testUserId';
173174

175+
$this->appConfig->expects($this->any())
176+
->method('getMimeTypes')
177+
->willReturn(Capabilities::MIMETYPES);
178+
174179
$this->config
175-
->expects($this->exactly(6))
176180
->method('getAppValue')
177181
->willReturnMap([
178182
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_enabled', 'no', 'yes'],
@@ -183,13 +187,55 @@ public function testShouldWatermarkOptionAllTags(array $objectTagIds, array $wat
183187
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_allTags', 'no', 'yes'],
184188
]);
185189

186-
$node->expects($this->once())->method('getId')->willReturn('testFileId');
187-
$node->expects($this->once())->method('isUpdateable')->willReturn(false);
188190
$this->systemTagMapper->expects($this->once())->method('getTagIdsForObjects')->willReturn(['testFileId' => $objectTagIds]);
189191
$this->appConfig->expects($this->once())->method('getAppValueArray')->willReturn($watermarkTagIds);
190192

191193
$result = $this->permissionManager->shouldWatermark($node, $userId, null);
192194

193195
$this->assertTrue($result);
194196
}
197+
198+
public function testShouldWatermarkOptionTalkPublic(): void {
199+
$node = $this->createNodeMock();
200+
$share = $this->createShareMock(IShare::TYPE_ROOM);
201+
$userId = null;
202+
203+
$this->appConfig->expects($this->any())
204+
->method('getMimeTypes')
205+
->willReturn(Capabilities::MIMETYPES);
206+
207+
$this->config
208+
->method('getAppValue')
209+
->willReturnMap([
210+
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_enabled', 'no', 'yes'],
211+
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkAll', 'no', 'no'],
212+
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkRead', 'no', 'no'],
213+
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkSecure', 'no', 'no'],
214+
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkTags', 'no', 'no'],
215+
[AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_shareTalkPublic', 'no', 'yes'],
216+
]);
217+
$result = $this->permissionManager->shouldWatermark($node, $userId, $share);
218+
219+
$this->assertTrue($result);
220+
}
221+
222+
private function createNodeMock(): Node {
223+
$node = $this->createMock(Node::class);
224+
$node->expects($this->any())->method('getMimetype')->willReturn('application/vnd.oasis.opendocument.spreadsheet');
225+
$node->expects($this->any())->method('getId')->willReturn('testFileId');
226+
$node->expects($this->any())->method('isUpdateable')->willReturn(false);
227+
return $node;
228+
}
229+
230+
private function createShareMock(?int $shareType): ?IShare {
231+
if ($shareType === null) {
232+
return null;
233+
}
234+
235+
$share = $this->createMock(IShare::class);
236+
$share->expects($this->any())->method('getHideDownload')->willReturn(true);
237+
$share->expects($this->any())->method('getAttributes')->willReturn(null);
238+
$share->expects($this->any())->method('getShareType')->willReturn($shareType);
239+
return $share;
240+
}
195241
}

‎tests/psalm-baseline.xml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<?xml version="1.0" encoding="UTF-8"?>
2-
<files psalm-version="5.26.1@d747f6500b38ac4f7dfc5edbcae6e4b637d7add0">
2+
<files psalm-version="6.5.1@3f17a6b24a2dbe543e21408c2b19108cf6a355ef">
33
<file src="lib/AppConfig.php">
44
<InvalidArgument>
55
<code><![CDATA[[]]]></code>
@@ -8,6 +8,11 @@
88
<code><![CDATA[array_key_exists($key, self::APP_SETTING_TYPES) && self::APP_SETTING_TYPES[$key] === 'array']]></code>
99
</RedundantCondition>
1010
</file>
11+
<file src="lib/AppInfo/Application.php">
12+
<MissingDependency>
13+
<code><![CDATA[SecureViewWrapper|IStorage]]></code>
14+
</MissingDependency>
15+
</file>
1116
<file src="lib/Command/ActivateConfig.php">
1217
<UndefinedClass>
1318
<code><![CDATA[Command]]></code>

‎tests/stub.phpstub‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,3 +130,20 @@ class OC_Util {
130130
public static function setupFS(?string $user);
131131
public static function tearDownFS();
132132
}
133+
134+
namespace OCA\Talk\Events {
135+
use OCP\EventDispatcher\Event;
136+
use OCP\Share\IShare;
137+
138+
class OverwritePublicSharePropertiesEvent extends Event {
139+
public function __construct(
140+
protected IShare $share,
141+
) {
142+
parent::__construct();
143+
}
144+
145+
public function getShare(): IShare {
146+
return $this->share;
147+
}
148+
}
149+
}

0 commit comments

Comments
 (0)