From 650fe7c725aed34c9c23a231f2c7280ace7074f8 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 28 Apr 2025 15:45:38 +0200 Subject: [PATCH] fix(Wopi): fall back to super share if share token is not available On internal shares the controller is called without the share token. But necessary information, like share attributes, might be necessary to know and are available from the super share of the SharedStorage in that case. For this approach was used elsewhere, too, some repetitive code was consolidated in the Helper class. Signed-off-by: Arthur Schiwon --- lib/Controller/AssetsController.php | 23 ++++++++++-------- lib/Controller/WopiController.php | 15 ++++++++---- lib/Helper.php | 17 +++++++++++++ lib/Listener/BeforeFetchPreviewListener.php | 27 ++++++++------------- lib/TokenManager.php | 22 ++++++----------- 5 files changed, 57 insertions(+), 47 deletions(-) diff --git a/lib/Controller/AssetsController.php b/lib/Controller/AssetsController.php index 90b73d3578..fe9dd29efa 100644 --- a/lib/Controller/AssetsController.php +++ b/lib/Controller/AssetsController.php @@ -6,9 +6,9 @@ namespace OCA\Richdocuments\Controller; -use OCA\Files_Sharing\SharedStorage; use OCA\Richdocuments\Controller\Attribute\RestrictToWopiServer; use OCA\Richdocuments\Db\AssetMapper; +use OCA\Richdocuments\Helper; use OCA\Richdocuments\Service\UserScopeService; use OCP\AppFramework\Controller; use OCP\AppFramework\Db\DoesNotExistException; @@ -30,13 +30,17 @@ class AssetsController extends Controller { private UserScopeService $userScopeService; private IURLGenerator $urlGenerator; + private Helper $helper; + public function __construct($appName, IRequest $request, AssetMapper $assetMapper, IRootFolder $rootFolder, $userId, UserScopeService $userScopeService, - IURLGenerator $urlGenerator) { + IURLGenerator $urlGenerator, + Helper $helper, + ) { parent::__construct($appName, $request); $this->assetMapper = $assetMapper; @@ -44,6 +48,7 @@ public function __construct($appName, $this->userId = $userId; $this->userScopeService = $userScopeService; $this->urlGenerator = $urlGenerator; + $this->helper = $helper; } /** @@ -63,14 +68,12 @@ public function create($path) { return new JSONResponse([], Http::STATUS_NOT_FOUND); } - $storage = $node->getStorage(); - if ($storage->instanceOfStorage(SharedStorage::class)) { - /** @var SharedStorage $storage */ - $share = $storage->getShare(); - $attributes = $share->getAttributes(); - if ($attributes !== null && $attributes->getAttribute('permissions', 'download') === false) { - throw new NotPermittedException(); - } + $share = $this->helper->getShareFromNode($node); + $attributes = $share?->getAttributes(); + if ($attributes !== null + && $attributes->getAttribute('permissions', 'download') === false + ) { + throw new NotPermittedException(); } } catch (NotFoundException $e) { return new JSONResponse([], Http::STATUS_NOT_FOUND); diff --git a/lib/Controller/WopiController.php b/lib/Controller/WopiController.php index 7dd81fe425..333b08d2fc 100644 --- a/lib/Controller/WopiController.php +++ b/lib/Controller/WopiController.php @@ -82,7 +82,8 @@ public function __construct( private IEncryptionManager $encryptionManager, private IGroupManager $groupManager, private ILockManager $lockManager, - private IEventDispatcher $eventDispatcher + private IEventDispatcher $eventDispatcher, + private Helper $helper, ) { parent::__construct($appName, $request); } @@ -187,7 +188,7 @@ public function checkFileInfo($fileId, $access_token) { $response['TemplateSource'] = $this->getWopiUrlForTemplate($wopi); } - $share = $this->getShareForWopiToken($wopi); + $share = $this->getShareForWopiToken($wopi, $file); if ($this->permissionManager->shouldWatermark($file, $wopi->getEditorUid(), $share)) { $email = $user !== null && !$isPublic ? $user->getEMailAddress() : ''; $replacements = [ @@ -804,10 +805,14 @@ private function getFileForWopiToken(Wopi $wopi) { return array_shift($files); } - private function getShareForWopiToken(Wopi $wopi): ?IShare { + private function getShareForWopiToken(Wopi $wopi, File $file): ?IShare { try { - return $wopi->getShare() ? $this->shareManager->getShareByToken($wopi->getShare()) : null; - } catch (ShareNotFound $e) { + $shareToken = $wopi->getShare(); + if ($shareToken) { + return $this->shareManager->getShareByToken($shareToken); + } + return $this->helper->getShareFromNode($file); + } catch (ShareNotFound) { } return null; diff --git a/lib/Helper.php b/lib/Helper.php index 50ba5dc5b1..e24e5bb2e0 100644 --- a/lib/Helper.php +++ b/lib/Helper.php @@ -7,7 +7,11 @@ use DateTime; use DateTimeZone; +use OCA\Files_Sharing\SharedStorage; use OCP\Files\Folder; +use OCP\Files\Node; +use OCP\Files\NotFoundException; +use OCP\Share\IShare; class Helper { /** @var string|null */ @@ -81,4 +85,17 @@ public function getGuestNameFromCookie() { } return $_COOKIE['guestUser']; } + + public function getShareFromNode(Node $node): ?IShare { + try { + $storage = $node->getStorage(); + } catch (NotFoundException) { + return null; + } + if ($storage->instanceOfStorage(SharedStorage::class)) { + /** @var SharedStorage $storage */ + return $storage->getShare(); + } + return null; + } } diff --git a/lib/Listener/BeforeFetchPreviewListener.php b/lib/Listener/BeforeFetchPreviewListener.php index 0adf48053c..cbff34597c 100644 --- a/lib/Listener/BeforeFetchPreviewListener.php +++ b/lib/Listener/BeforeFetchPreviewListener.php @@ -10,7 +10,7 @@ namespace OCA\Richdocuments\Listener; -use OCA\Files_Sharing\SharedStorage; +use OCA\Richdocuments\Helper; use OCA\Richdocuments\PermissionManager; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; @@ -20,7 +20,6 @@ use OCP\Preview\BeforePreviewFetchedEvent; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; -use OCP\Share\IShare; /** @template-implements IEventListener */ class BeforeFetchPreviewListener implements IEventListener { @@ -28,12 +27,14 @@ class BeforeFetchPreviewListener implements IEventListener { private IUserSession $userSession; private IRequest $request; private IManager $shareManager; + private Helper $helper; - public function __construct(PermissionManager $permissionManager, IUserSession $userSession, IRequest $request, IManager $shareManager) { + public function __construct(PermissionManager $permissionManager, IUserSession $userSession, IRequest $request, IManager $shareManager, Helper $helper) { $this->permissionManager = $permissionManager; $this->userSession = $userSession; $this->request = $request; $this->shareManager = $shareManager; + $this->helper = $helper; } public function handle(Event $event): void { @@ -42,21 +43,13 @@ public function handle(Event $event): void { } $shareToken = $this->request->getParam('token'); - $share = null; - - // Get share for internal shares - $storage = $event->getNode()->getStorage(); - if (!$shareToken && $storage->instanceOfStorage(SharedStorage::class)) { - if (method_exists(IShare::class, 'getAttributes')) { - /** @var SharedStorage $storage */ - $share = $storage->getShare(); - } - } - - // Get different share for public previews as the share from the node is only set for mounted shares try { - $share = $shareToken ? $this->shareManager->getShareByToken($shareToken) : $share; - } catch (ShareNotFound $e) { + $share = $shareToken ? + // Get different share for public previews as the share from the node is only set for mounted shares + $this->shareManager->getShareByToken($shareToken) + // Get share for internal shares + : $this->helper->getShareFromNode($event->getNode()); + } catch (ShareNotFound) { } $userId = $this->userSession->getUser() ? $this->userSession->getUser()->getUID() : null; diff --git a/lib/TokenManager.php b/lib/TokenManager.php index 245ea0a281..1fbb066ea1 100644 --- a/lib/TokenManager.php +++ b/lib/TokenManager.php @@ -7,7 +7,6 @@ namespace OCA\Richdocuments; use Exception; -use OCA\Files_Sharing\SharedStorage; use OCA\Richdocuments\Db\Direct; use OCA\Richdocuments\Db\Wopi; use OCA\Richdocuments\Db\WopiMapper; @@ -24,7 +23,6 @@ use OCP\IURLGenerator; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; -use OCP\Share\IShare; use OCP\Util; use Psr\Log\LoggerInterface; @@ -83,19 +81,13 @@ public function generateWopiToken(string $fileId, ?string $shareToken = null, ?s // disable download if at least one shared access has it disabled foreach ($files as $file) { - $storage = $file->getStorage(); - // using string as we have no guarantee that "files_sharing" app is loaded - if ($storage->instanceOfStorage(SharedStorage::class)) { - if (!method_exists(IShare::class, 'getAttributes')) { - break; - } - /** @var SharedStorage $storage */ - $share = $storage->getShare(); - $attributes = $share->getAttributes(); - if ($attributes !== null && $attributes->getAttribute('permissions', 'download') === false) { - $hideDownload = true; - break; - } + $share = $this->helper->getShareFromNode($file); + $attributes = $share?->getAttributes(); + if ($attributes !== null + && $attributes->getAttribute('permissions', 'download') === false + ) { + $hideDownload = true; + break; } } }