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 <blizzz@arthur-schiwon.de>
This commit is contained in:
Arthur Schiwon 2025-11-13 22:28:20 +01:00
parent 91b6659028
commit b62b572818
No known key found for this signature in database
GPG key ID: 7424F1874854DF23
10 changed files with 405 additions and 38 deletions

View file

@ -8,6 +8,7 @@ declare(strict_types=1);
namespace OCA\Richdocuments\AppInfo;
use OCA\DAV\Events\SabrePluginAddEvent;
use OCA\Files_Sharing\Event\ShareLinkAccessedEvent;
use OCA\Richdocuments\AppConfig;
use OCA\Richdocuments\Capabilities;
@ -15,6 +16,7 @@ use OCA\Richdocuments\Conversion\ConversionProvider;
use OCA\Richdocuments\Db\WopiMapper;
use OCA\Richdocuments\Listener\AddContentSecurityPolicyListener;
use OCA\Richdocuments\Listener\AddFeaturePolicyListener;
use OCA\Richdocuments\Listener\AddSabrePluginListener;
use OCA\Richdocuments\Listener\BeforeFetchPreviewListener;
use OCA\Richdocuments\Listener\BeforeGetTemplatesListener;
use OCA\Richdocuments\Listener\BeforeTemplateRenderedListener;
@ -49,6 +51,7 @@ use OCP\AppFramework\Bootstrap\IBootContext;
use OCP\AppFramework\Bootstrap\IBootstrap;
use OCP\AppFramework\Bootstrap\IRegistrationContext;
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
use OCP\BeforeSabrePubliclyLoadedEvent;
use OCP\Collaboration\Reference\RenderReferenceEvent;
use OCP\Collaboration\Resources\LoadAdditionalScriptsEvent;
use OCP\Files\Storage\IStorage;
@ -86,6 +89,8 @@ class Application extends App implements IBootstrap {
$context->registerEventListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class);
$context->registerEventListener(BeforeGetTemplatesEvent::class, BeforeGetTemplatesListener::class);
$context->registerEventListener(OverwritePublicSharePropertiesEvent::class, OverwritePublicSharePropertiesListener::class);
$context->registerEventListener(SabrePluginAddEvent::class, AddSabrePluginListener::class);
$context->registerEventListener(BeforeSabrePubliclyLoadedEvent::class, AddSabrePluginListener::class);
$context->registerReferenceProvider(OfficeTargetReferenceProvider::class);
$context->registerSensitiveMethods(WopiMapper::class, [
'getPathForToken',

View file

@ -0,0 +1,85 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\Richdocuments\DAV;
use OCA\DAV\Connector\Sabre\FilesPlugin;
use OCA\DAV\Connector\Sabre\Node;
use OCA\Richdocuments\Middleware\WOPIMiddleware;
use OCA\Richdocuments\Service\SecureViewService;
use OCA\Richdocuments\Storage\SecureViewWrapper;
use OCP\Files\ForbiddenException;
use OCP\Files\NotFoundException;
use OCP\Files\StorageNotAvailableException;
use OCP\IAppConfig;
use Psr\Log\LoggerInterface;
use Sabre\DAV\INode;
use Sabre\DAV\PropFind;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
class SecureViewPlugin extends ServerPlugin {
public function __construct(
protected WOPIMiddleware $wopiMiddleware,
protected IAppConfig $appConfig,
protected SecureViewService $secureViewService,
protected LoggerInterface $logger,
) {
}
public function initialize(Server $server) {
if (!$this->secureViewService->isEnabled()) {
return;
}
$server->on('propFind', $this->handleGetProperties(...));
}
private function handleGetProperties(PropFind $propFind, INode $node): void {
if (!$node instanceof Node) {
return;
}
$requestedProperties = $propFind->getRequestedProperties();
if (!in_array(FilesPlugin::SHARE_HIDE_DOWNLOAD_PROPERTYNAME, $requestedProperties, true)) {
return;
}
$currentValue = $propFind->get(FilesPlugin::SHARE_HIDE_DOWNLOAD_PROPERTYNAME);
if ($currentValue === 'true') {
// We won't unhide, hence can return early
return;
}
if (!$this->isDownloadable($node->getNode())) {
// 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?
$propFind->set(FilesPlugin::SHARE_HIDE_DOWNLOAD_PROPERTYNAME, 'true');
// avoid potential race condition with FilesPlugin that may set it to "false"
$propFind->handle(FilesPlugin::SHARE_HIDE_DOWNLOAD_PROPERTYNAME, 'true');
}
}
private function isDownloadable(\OCP\Files\Node $node): bool {
$storage = $node->getStorage();
if ($this->wopiMiddleware->isWOPIRequest()
|| $storage === null
|| !$storage->instanceOfStorage(SecureViewWrapper::class)
) {
return true;
}
try {
return !$this->secureViewService->shouldSecure($node->getInternalPath(), $storage);
} catch (StorageNotAvailableException|ForbiddenException|NotFoundException $e) {
// Exceptions cannot be nicely inferred.
return false;
} catch (\Throwable $e) {
$this->logger->warning('SecureViewPlugin caught an exception that likely is ignorable. Still preventing download.',
['exception' => $e,]
);
return false;
}
}
}

View file

@ -0,0 +1,35 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\Richdocuments\Listener;
use OCA\DAV\Events\SabrePluginAddEvent;
use OCA\Richdocuments\DAV\SecureViewPlugin;
use OCP\BeforeSabrePubliclyLoadedEvent;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
use Psr\Container\ContainerInterface;
/** @template-implements IEventListener<SabrePluginAddEvent|BeforeSabrePubliclyLoadedEvent> */
class AddSabrePluginListener implements IEventListener {
public function __construct(
protected ContainerInterface $server,
) {
}
public function handle(Event $event): void {
if (
!$event instanceof SabrePluginAddEvent
&& !$event instanceof BeforeSabrePubliclyLoadedEvent
) {
return;
}
$davServer = $event->getServer();
$davServer->addPlugin($this->server->get(SecureViewPlugin::class));
}
}

View file

@ -0,0 +1,60 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\Richdocuments\Service;
use OCA\Richdocuments\AppConfig;
use OCA\Richdocuments\PermissionManager;
use OCP\Files\NotFoundException;
use OCP\Files\Storage\ISharedStorage;
use OCP\Files\Storage\IStorage;
use OCP\IAppConfig;
use OCP\IUserSession;
class SecureViewService {
public function __construct(
protected IUserSession $userSession,
protected PermissionManager $permissionManager,
protected IAppConfig $appConfig,
) {
}
public function isEnabled(): bool {
return $this->appConfig->getValueString(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_enabled', 'no') !== 'no';
}
/**
* @throws NotFoundException
*/
public function shouldSecure(string $path, IStorage $storage, bool $tryOpen = true): bool {
if ($tryOpen) {
// pity… fopen() does not document any possible Exceptions
$fp = $storage->fopen($path, 'r');
fclose($fp);
}
$cacheEntry = $storage->getCache()->get($path);
if (!$cacheEntry) {
$parent = dirname($path);
if ($parent === '.') {
$parent = '';
}
$cacheEntry = $storage->getCache()->get($parent);
if (!$cacheEntry) {
throw new NotFoundException(sprintf('Could not find cache entry for path and parent of %s within storage %s ', $path, $storage->getId()));
}
}
$isSharedStorage = $storage->instanceOfStorage(ISharedStorage::class);
/** @noinspection PhpPossiblePolymorphicInvocationInspection */
/** @psalm-suppress UndefinedMethod **/
$share = $isSharedStorage ? $storage->getShare() : null;
$userId = $this->userSession->getUser()?->getUID();
return $this->permissionManager->shouldWatermark($cacheEntry, $userId, $share, $storage->getOwner($path) ?: null);
}
}

View file

@ -11,10 +11,9 @@ namespace OCA\Richdocuments\Storage;
use OC\Files\Storage\Wrapper\Wrapper;
use OCA\Richdocuments\Middleware\WOPIMiddleware;
use OCA\Richdocuments\PermissionManager;
use OCA\Richdocuments\Service\SecureViewService;
use OCP\Files\ForbiddenException;
use OCP\Files\IRootFolder;
use OCP\Files\NotFoundException;
use OCP\Files\Storage\ISharedStorage;
use OCP\Files\Storage\IStorage;
use OCP\IUserSession;
use OCP\Server;
@ -24,6 +23,7 @@ class SecureViewWrapper extends Wrapper {
private WOPIMiddleware $wopiMiddleware;
private IRootFolder $rootFolder;
private IUserSession $userSession;
private SecureViewService $secureViewService;
private string $mountPoint;
@ -34,6 +34,7 @@ class SecureViewWrapper extends Wrapper {
$this->wopiMiddleware = Server::get(WOPIMiddleware::class);
$this->rootFolder = Server::get(IRootFolder::class);
$this->userSession = Server::get(IUserSession::class);
$this->secureViewService = Server::get(SecureViewService::class);
$this->mountPoint = $parameters['mountPoint'];
}
@ -78,41 +79,15 @@ class SecureViewWrapper extends Wrapper {
* @throws ForbiddenException
*/
private function checkFileAccess(string $path): void {
if ($this->shouldSecure($path) && !$this->wopiMiddleware->isWOPIRequest()) {
if (!$this->wopiMiddleware->isWOPIRequest() && $this->secureViewService->shouldSecure($path, $this, false)) {
throw new ForbiddenException('Download blocked due the secure view policy', false);
}
}
private function shouldSecure(string $path, ?IStorage $sourceStorage = null): bool {
if ($sourceStorage !== $this && $sourceStorage !== null) {
$fp = $sourceStorage->fopen($path, 'r');
fclose($fp);
}
$storage = $sourceStorage ?? $this;
$cacheEntry = $storage->getCache()->get($path);
if (!$cacheEntry) {
$parent = dirname($path);
if ($parent === '.') {
$parent = '';
}
$cacheEntry = $storage->getCache()->get($parent);
if (!$cacheEntry) {
throw new NotFoundException(sprintf('Could not find cache entry for path and parent of %s within storage %s ', $path, $storage->getId()));
}
}
$isSharedStorage = $storage->instanceOfStorage(ISharedStorage::class);
$share = $isSharedStorage ? $storage->getShare() : null;
$userId = $this->userSession->getUser()?->getUID();
return $this->permissionManager->shouldWatermark($cacheEntry, $userId, $share, $storage->getOwner($path) ?: null);
}
private function checkSourceAndTarget(string $source, string $target, ?IStorage $sourceStorage = null): void {
if ($this->shouldSecure($source, $sourceStorage) && !$this->shouldSecure($target)) {
if ($this->secureViewService->shouldSecure($source, $sourceStorage ?? $this, $sourceStorage !== null)
&& !$this->secureViewService->shouldSecure($target, $this)
) {
throw new ForbiddenException('Download blocked due the secure view policy. The source requires secure view that the target cannot offer.', false);
}
}

View file

@ -30,6 +30,7 @@ class RichDocumentsContext implements Context {
private $fileIds = [];
/** @var array List of templates fetched for a given file type */
private $templates = [];
private array $directoryListing = [];
/** @BeforeScenario */
public function gatherContexts(BeforeScenarioScope $scope) {
@ -75,6 +76,47 @@ class RichDocumentsContext implements Context {
Assert::assertNotEmpty($this->wopiToken);
}
/**
* @When the download button for :path will be visible to :user
*/
public function downloadButtonIsVisible(string $path, string $user): void {
$this->downloadButtonIsNotVisibleOrNot($path, $user, true);
}
/**
* @When the download button for :path will not be visible to :user
*/
public function downloadButtonIsNotVisible(string $path, string $user): void {
$this->downloadButtonIsNotVisibleOrNot($path, $user, false);
}
private function downloadButtonIsNotVisibleOrNot(string $path, string $user, bool $isVisible): void {
$hideDownloadProperty = '{http://nextcloud.org/ns}hide-download';
$this->serverContext->usingWebAsUser($user);
$fileInfo = $this->filesContext->listFolder($path, 0, [$hideDownloadProperty]);
if ($isVisible) {
Assert::assertTrue(!isset($fileInfo[$hideDownloadProperty]) || $fileInfo[$hideDownloadProperty] === 'false');
} else {
Assert::assertTrue(isset($fileInfo[$hideDownloadProperty]), 'property is not set');
Assert::assertTrue($fileInfo[$hideDownloadProperty] === 'true', 'property is not true');
Assert::assertTrue(isset($fileInfo[$hideDownloadProperty]) && $fileInfo[$hideDownloadProperty] === 'true');
}
}
/**
* @When the download button for :path will not be visible in the last link share
*/
public function theDownloadButtonWillNotBeVisibleInLastLinkShare(string $path): void {
$hideDownloadProperty = '{http://nextcloud.org/ns}hide-download';
$this->serverContext->usingWebAsUser();
$shareToken = $this->sharingContext->getLastShareData()['token'];
$davClient = $this->filesContext->getPublicSabreClient($shareToken);
$result = $davClient->propFind($path, ['{http://nextcloud.org/ns}hide-download'], 1);
$fileInfo = $result[array_key_first($result)];
Assert::assertTrue(!isset($fileInfo[$hideDownloadProperty]) || $fileInfo[$hideDownloadProperty] === 'true');
}
public function generateTokenWithApi($user, $fileId, ?string $shareToken = null, ?string $path = null, ?string $guestName = null) {
$this->serverContext->usingWebAsUser($user);
$this->serverContext->sendJSONRequest('POST', '/index.php/apps/richdocuments/token', [
@ -248,4 +290,26 @@ class RichDocumentsContext implements Context {
$this->wopiContext->collaboraRenamesTo($fileId, $newName);
}
/**
* @When admin enables secure view
*/
public function enableSecureView(): void {
$this->serverContext->actAsAdmin(function () {
$watermarkKeysToEnable = [
'watermark_enabled',
'watermark_linkAll',
'watermark_shareRead',
];
foreach ($watermarkKeysToEnable as $configKey) {
$this->serverContext->sendOCSRequest(
'POST',
'apps/provisioning_api/api/v1/config/apps/files/' . $configKey,
['value' => 'yes'],
);
$this->serverContext->assertHttpStatusCode(200);
}
});
}
}

View file

@ -0,0 +1,38 @@
Feature: API
Background:
Given user "user1" exists
And user "user2" exists
And admin enables secure view
Scenario: Download button is not shown in public shares
Given as user "user1"
And User "user1" creates a folder "NewFolder"
And User "user1" uploads file "./../emptyTemplates/template.odt" to "/NewFolder/file.odt"
And as "user1" create a share with
| path | /NewFolder |
| shareType | 3 |
Then the download button for "/NewFolder/file.odt" will be visible to "user1"
And the download button for "file.odt" will not be visible in the last link share
Scenario: Download button is not shown in internal read-only shares
Given as user "user1"
And User "user1" creates a folder "NewFolder"
And User "user1" uploads file "./../emptyTemplates/template.odt" to "/NewFolder/file.odt"
And as "user1" create a share with
| path | /NewFolder |
| shareType | 0 |
| shareWith | user2 |
| permissions | 1 |
Then the download button for "/NewFolder/file.odt" will not be visible to "user2"
Scenario: Download button is shown in internal shares
Given as user "user1"
And User "user1" creates a folder "NewFolder"
And User "user1" uploads file "./../emptyTemplates/template.odt" to "/NewFolder/file.odt"
And as "user1" create a share with
| path | /NewFolder |
| shareType | 0 |
| shareWith | user2 |
| permissions | 31 |
Then the download button for "/NewFolder/file.odt" will be visible to "user2"

View file

@ -8,11 +8,6 @@
<code><![CDATA[array_key_exists($key, self::APP_SETTING_TYPES) && self::APP_SETTING_TYPES[$key] === 'array']]></code>
</RedundantCondition>
</file>
<file src="lib/AppInfo/Application.php">
<MissingDependency>
<code><![CDATA[SecureViewWrapper|IStorage]]></code>
</MissingDependency>
</file>
<file src="lib/Command/ActivateConfig.php">
<UndefinedClass>
<code><![CDATA[Command]]></code>

View file

@ -55,7 +55,6 @@ $OCC config:system:set allow_local_remote_servers --value true --type bool
$OCC config:system:set gs.trustedHosts 0 --value="localhost:$PORT_SERVERA"
$OCC config:system:set gs.trustedHosts 1 --value="localhost:$PORT_SERVERB"
vendor/bin/behat $SCENARIO_TO_RUN
RESULT=$?

View file

@ -10,6 +10,76 @@ class OC_User {
public static function setIncognitoMode($status) {}
}
namespace OC\Files\Storage\Wrapper {
class Wrapper implements \OCP\Files\Storage\IStorage{
protected $storage;
public function __construct(array $parameters) {}
public function copy(string $source, string $target): bool {}
public function copyFromStorage(\OCP\Files\Storage\IStorage $sourceStorage, string $sourceInternalPath, string $targetInternalPath): bool {}
public function moveFromStorage(\OCP\Files\Storage\IStorage $sourceStorage, string $sourceInternalPath, string $targetInternalPath): bool {}
public function rename(string $source, string $target): bool {}
public function getId(): string {};
public function mkdir(string $path): bool {};
public function rmdir(string $path): bool {};
public function opendir(string $path) {};
public function is_dir(string $path): bool {};
public function is_file(string $path): bool {};
public function is_file(string $path): bool {};
public function stat(string $path): array|false {};
public function filetype(string $path): string|false {};
public function filesize(string $path): int|float|false {};
public function isCreatable(string $path): bool {};
public function isReadable(string $path): bool {};
public function isUpdatable(string $path): bool {};
public function isDeletable(string $path): bool {};
public function isSharable(string $path): bool {};
public function getPermissions(string $path): int {};
public function file_exists(string $path): bool {};
public function filemtime(string $path): int|false {};
public function file_get_contents(string $path): string|false {};
public function file_put_contents(string $path, mixed $data): int|float|false {};
public function unlink(string $path): bool {};
public function rename(string $source, string $target): bool {};
public function copy(string $source, string $target): bool {};
public function fopen(string $path, string $mode) {};
public function getMimeType(string $path): string|false {};
public function hash(string $type, string $path, bool $raw = false): string|false {};
public function free_space(string $path): int|float|false {};
public function touch(string $path, ?int $mtime = null): bool {};
public function getLocalFile(string $path): string|false {};
public function hasUpdated(string $path, int $time): bool {};
public function getCache(string $path = , ?IStorage $storage = null): ICache {};
public function getScanner(string $path = , ?IStorage $storage = null): IScanner {};
public function getOwner(string $path): string|false {};
public function getWatcher(string $path = , ?IStorage $storage = null): IWatcher {};
public function getPropagator(?IStorage $storage = null): IPropagator {};
public function getUpdater(?IStorage $storage = null): IUpdater {};
public function getStorageCache(): OCFilesCacheStorage {};
public function getETag(string $path): string|false {};
public function test(): bool {};
public function isLocal(): bool {};
public function instanceOfStorage(string $class): bool {};
public function getInstanceOfStorage(string $class): ?IStorage {};
public function __call(string $method, array $args) {};
public function getDirectDownload(string $path): array|false {};
public function getAvailability(): array {};
public function setAvailability(bool $isAvailable): void {};
public function verifyPath(string $path, string $fileName): void {};
public function copyFromStorage(\OCP\Files\Storage\IStorage $sourceStorage, string $sourceInternalPath, string $targetInternalPath): bool {};
public function moveFromStorage(\OCP\Files\Storage\IStorage $sourceStorage, string $sourceInternalPath, string $targetInternalPath): bool {};
public function getMetaData(string $path): ?array {};
public function acquireLock(string $path, int $type, ILockingProvider $provider): void {};
public function releaseLock(string $path, int $type, ILockingProvider $provider): void {};
public function changeLock(string $path, int $type, ILockingProvider $provider): void {};
public function needsPartFile(): bool {};
public function writeStream(string $path, $stream, ?int $size = null): int {};
public function getDirectoryContent(string $directory): Traversable {};
public function isWrapperOf(IStorage $storage): bool {};
public function setOwner(?string $user): void {};
}
}
namespace OC\Hooks {
interface Emitter {
/**
@ -32,6 +102,20 @@ namespace OC\Hooks {
}
}
namespace OCA\DAV\Connector\Sabre {
abstract class Node {
public function getNode(): \OCP\Files\Node;
}
class FilesPlugin {
public const SHARE_HIDE_DOWNLOAD_PROPERTYNAME = 'somePropertyName';
}
}
namespace OCA\DAV\Events {
class SabrePluginAddEvent extends \OCP\EventDispatcher\Event {}
}
namespace OCA\Federation {
class TrustedServers {
public function getServers() {
@ -147,3 +231,30 @@ namespace OCA\Talk\Events {
}
}
}
namespace Sabre\DAV {
interface INode {}
class PropFind {
/**
* @return string[]
*/
public function getRequestedProperties(): array {
return [];
}
public function get(string $key) {
return null;
}
public function set(string $key, $value) {}
public function handle(string $key, $valueOrCallable): void {}
}
class Server {
public function on(string $eventName, callable $callable): void {}
}
class ServerPlugin {}
}