[stable31] fix(SecureView): hide disfunctional *download* files action

lib/AppInfo/Application.php

@@ -8,13 +8,15 @@
 
 namespace OCA\Richdocuments\AppInfo;
 
+use OCA\DAV\Events\SabrePluginAddEvent;
 use OCA\Files_Sharing\Event\ShareLinkAccessedEvent;
 use OCA\Richdocuments\AppConfig;
 use OCA\Richdocuments\Capabilities;
 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;
@@ -43,6 +45,7 @@
 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;
@@ -80,6 +83,8 @@ public function register(IRegistrationContext $context): void {
 		$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',

lib/DAV/SecureViewPlugin.php

@@ -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;
+		}
+	}
+}

lib/Listener/AddSabrePluginListener.php

@@ -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));
+	}
+}

lib/Service/SecureViewService.php

@@ -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);
+	}
+}

lib/Storage/SecureViewWrapper.php

@@ -11,10 +11,9 @@
 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 @@ public function __construct(array $parameters) {
 		$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 @@ public function rename(string $source, string $target): bool {
 	 * @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);
 		}
 	}

tests/features/bootstrap/RichDocumentsContext.php

@@ -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 @@ public function userOpens($user, $file) {
 		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 @@ public function renameFileTo($user, $file, $newName) {
 
 		$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);
+			}
+		});
+	}
 }