From d1964891fdc10795b7c1e1ebf5b7e35e65cde38d Mon Sep 17 00:00:00 2001 From: Elizabeth Danzberger Date: Wed, 13 Nov 2024 12:04:51 -0500 Subject: [PATCH] fix: do not treat guest as share owner Signed-off-by: Elizabeth Danzberger --- lib/Controller/DirectViewController.php | 2 +- lib/Controller/DocumentController.php | 19 ++++++-- lib/Controller/WopiController.php | 40 ++++++---------- lib/Db/WopiMapper.php | 4 ++ lib/TokenManager.php | 60 +++++++++++++++++++----- tests/features/bootstrap/WopiContext.php | 30 ++++++++++++ tests/features/wopi.feature | 16 +++++++ tests/stub.phpstub | 4 ++ 8 files changed, 134 insertions(+), 41 deletions(-) diff --git a/lib/Controller/DirectViewController.php b/lib/Controller/DirectViewController.php index 5ca21b648a..e81e3454ca 100644 --- a/lib/Controller/DirectViewController.php +++ b/lib/Controller/DirectViewController.php @@ -103,7 +103,7 @@ public function show($token) { try { $urlSrc = $this->tokenManager->getUrlSrc($item); - $wopi = $this->tokenManager->generateWopiTokenForTemplate($item, $direct->getUid(), $direct->getTemplateDestination(), true); + $wopi = $this->tokenManager->generateWopiTokenForTemplate($item, $direct->getTemplateDestination(), $direct->getUid(), false, true); $targetFile = $folder->getById($direct->getTemplateDestination())[0]; $relativePath = $folder->getRelativePath($targetFile->getPath()); diff --git a/lib/Controller/DocumentController.php b/lib/Controller/DocumentController.php index 72029d9cca..53390fba52 100644 --- a/lib/Controller/DocumentController.php +++ b/lib/Controller/DocumentController.php @@ -187,7 +187,8 @@ public function createFromTemplate(int $templateId, string $fileName, string $di $template = $this->templateManager->get($templateId); $urlSrc = $this->tokenManager->getUrlSrc($file); - $wopi = $this->tokenManager->generateWopiTokenForTemplate($template, $this->userId, $file->getId()); + $isGuest = $this->userId === null; + $wopi = $this->tokenManager->generateWopiTokenForTemplate($template, $file->getId(), $this->userId, $isGuest); $params = [ 'permissions' => $template->getPermissions(), @@ -400,7 +401,8 @@ public function token(int $fileId, ?string $shareToken = null, ?string $path = n ]); } - $wopi = $this->getToken($file, $share); + $isGuest = $guestName || !$this->userId; + $wopi = $this->getToken($file, $share, null, $isGuest); $this->tokenManager->setGuestName($wopi, $guestName); @@ -485,11 +487,20 @@ private function getFileForShare(IShare $share, ?int $fileId, ?string $path = nu throw new NotFoundException(); } - private function getToken(File $file, ?IShare $share = null, int $version = null): Wopi { + private function getToken(File $file, ?IShare $share = null, ?int $version = null, bool $isGuest = false): Wopi { // Pass through $version $templateFile = $this->templateManager->getTemplateSource($file->getId()); if ($templateFile) { - return $this->tokenManager->generateWopiTokenForTemplate($templateFile, $share?->getShareOwner() ?? $this->userId, $file->getId()); + $owneruid = $share?->getShareOwner() ?? $file->getOwner()->getUID(); + + return $this->tokenManager->generateWopiTokenForTemplate( + $templateFile, + $file->getId(), + $owneruid, + $isGuest, + false, + $share?->getPermissions() + ); } return $this->tokenManager->generateWopiToken($this->getWopiFileId($file->getId(), $version), $share?->getToken(), $this->userId); diff --git a/lib/Controller/WopiController.php b/lib/Controller/WopiController.php index 1d0bc12121..81867221f2 100644 --- a/lib/Controller/WopiController.php +++ b/lib/Controller/WopiController.php @@ -611,7 +611,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse { // Unless the editor is empty (public link) we modify the files as the current editor $editor = $wopi->getEditorUid(); - if ($editor === null && !$wopi->isRemoteToken()) { + $isPublic = $editor === null && !$wopi->isRemoteToken(); + if ($isPublic) { $editor = $wopi->getOwnerUid(); } @@ -627,16 +628,10 @@ public function postFile(string $fileId, string $access_token): JSONResponse { $file = $this->getFileForWopiToken($wopi); $suggested = $this->request->getHeader('X-WOPI-RequestedName'); - $suggested = mb_convert_encoding($suggested, 'utf-8', 'utf-7') . '.' . $file->getExtension(); - if (strpos($suggested, '.') === 0) { - $path = dirname($file->getPath()) . '/New File' . $suggested; - } elseif (strpos($suggested, '/') !== 0) { - $path = dirname($file->getPath()) . '/' . $suggested; - } else { - $path = $userFolder->getPath() . $suggested; - } + $parent = $isPublic ? dirname($file->getPath()) : $userFolder->getPath(); + $path = $this->normalizePath($suggested, $parent); if ($path === '') { return new JSONResponse([ @@ -665,20 +660,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse { $suggested = $this->request->getHeader('X-WOPI-SuggestedTarget'); $suggested = mb_convert_encoding($suggested, 'utf-8', 'utf-7'); - if ($suggested[0] === '.') { - $path = dirname($file->getPath()) . '/New File' . $suggested; - } elseif ($suggested[0] !== '/') { - $path = dirname($file->getPath()) . '/' . $suggested; - } else { - $path = $userFolder->getPath() . $suggested; - } - - if ($path === '') { - return new JSONResponse([ - 'status' => 'error', - 'message' => 'Cannot create the file' - ]); - } + $parent = $isPublic ? dirname($file->getPath()) : $userFolder->getPath(); + $path = $this->normalizePath($suggested, $parent); // create the folder first if (!$this->rootFolder->nodeExists(dirname($path))) { @@ -692,8 +675,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse { $content = fopen('php://input', 'rb'); // Set the user to register the change under his name - $this->userScopeService->setUserScope($wopi->getEditorUid()); - $this->userScopeService->setFilesystemScope($wopi->getEditorUid()); + $this->userScopeService->setUserScope($editor); + $this->userScopeService->setFilesystemScope($editor); try { $this->wrappedFilesystemOperation($wopi, function () use ($file, $content) { @@ -722,6 +705,13 @@ public function postFile(string $fileId, string $access_token): JSONResponse { } } + private function normalizePath(string $path, ?string $parent = null): string { + $path = str_starts_with($path, '/') ? $path : '/' . $path; + $parent = is_null($parent) ? '' : rtrim($parent, '/'); + + return $parent . $path; + } + private function lock(Wopi $wopi, string $lock): JSONResponse { try { $lock = $this->lockManager->lock(new LockContext( diff --git a/lib/Db/WopiMapper.php b/lib/Db/WopiMapper.php index 7425b22dd5..82f9016453 100644 --- a/lib/Db/WopiMapper.php +++ b/lib/Db/WopiMapper.php @@ -68,6 +68,10 @@ public function __construct(IDBConnection $db, * @param string $serverHost * @param string $guestDisplayname * @param int $templateDestination + * @param bool $hideDownload + * @param bool $direct + * @param int $templateId + * @param string $share * @return Wopi */ public function generateFileToken($fileId, $owner, $editor, $version, $updatable, $serverHost, $guestDisplayname = null, $templateDestination = 0, $hideDownload = false, $direct = false, $templateId = 0, $share = null) { diff --git a/lib/TokenManager.php b/lib/TokenManager.php index 83593fe169..b8d740c925 100644 --- a/lib/TokenManager.php +++ b/lib/TokenManager.php @@ -41,6 +41,7 @@ use OCP\Share\IManager; use OCP\Share\IShare; use OCP\Util; +use Psr\Log\LoggerInterface; class TokenManager { /** @var IRootFolder */ @@ -63,6 +64,8 @@ class TokenManager { private $helper; /** @var PermissionManager */ private $permissionManager; + /** @var LoggerInterface */ + private $logger; public function __construct( IRootFolder $rootFolder, @@ -74,7 +77,8 @@ public function __construct( WopiMapper $wopiMapper, IL10N $trans, Helper $helper, - PermissionManager $permissionManager + PermissionManager $permissionManager, + LoggerInterface $logger ) { $this->rootFolder = $rootFolder; $this->shareManager = $shareManager; @@ -86,6 +90,7 @@ public function __construct( $this->wopiMapper = $wopiMapper; $this->helper = $helper; $this->permissionManager = $permissionManager; + $this->logger = $logger; } /** @@ -151,7 +156,7 @@ public function generateWopiToken(string $fileId, ?string $shareToken = null, ?s // no active user login while generating the token // this is required during WopiPutRelativeFile if (is_null($editoruid)) { - \OC::$server->getLogger()->warning('Generating token for SaveAs without editoruid'); + $this->logger->warning('Generating token for SaveAs without editoruid'); $updatable = true; } else { // Make sure we use the user folder if available since fetching all files by id from the root might be expensive @@ -237,12 +242,18 @@ public function upgradeFromDirectInitiator(Direct $direct, Wopi $wopi) { return $wopi; } - public function generateWopiTokenForTemplate(File $templateFile, ?string $userId, int $targetFileId, bool $direct = false): Wopi { - $owneruid = $userId; - $editoruid = $userId; - $rootFolder = $this->rootFolder->getUserFolder($editoruid); - $targetFile = $rootFolder->getById($targetFileId); - $targetFile = array_shift($targetFile); + public function generateWopiTokenForTemplate( + File $templateFile, + int $targetFileId, + string $owneruid, + bool $isGuest, + bool $direct = false, + ?int $sharePermissions = null, + ): Wopi { + $editoruid = $isGuest ? null : $owneruid; + + $rootFolder = $this->rootFolder->getUserFolder($owneruid); + $targetFile = $rootFolder->getById($targetFileId)[0]; if (!$targetFile instanceof File) { throw new NotFoundException(); } @@ -252,16 +263,43 @@ public function generateWopiTokenForTemplate(File $templateFile, ?string $userId throw new NotPermittedException(); } - $updatable = $targetFile->isUpdateable() && $this->permissionManager->userCanEdit($editoruid); + $updatable = $targetFile->isUpdateable(); + if (!is_null($sharePermissions)) { + $shareUpdatable = (bool)($sharePermissions & \OCP\Constants::PERMISSION_UPDATE); + $updatable = $updatable && $shareUpdatable; + } $serverHost = $this->urlGenerator->getAbsoluteURL('/'); if ($this->capabilitiesService->hasTemplateSource()) { - return $this->wopiMapper->generateFileToken($targetFile->getId(), $owneruid, $editoruid, 0, $updatable, $serverHost, null, 0, false, $direct, $templateFile->getId()); + return $this->wopiMapper->generateFileToken( + $targetFile->getId(), + $owneruid, + $editoruid, + 0, + $updatable, + $serverHost, + $isGuest ? '' : null, + 0, + false, + $direct, + $templateFile->getId() + ); } // Legacy way of creating new documents from a template - return $this->wopiMapper->generateFileToken($templateFile->getId(), $owneruid, $editoruid, 0, $updatable, $serverHost, null, $targetFile->getId(), $direct); + return $this->wopiMapper->generateFileToken( + $templateFile->getId(), + $owneruid, + $editoruid, + 0, + $updatable, + $serverHost, + $isGuest ? '' : null, + $targetFile->getId(), + false, + $direct + ); } public function newInitiatorToken($sourceServer, Node $node = null, $shareToken = null, bool $direct = false, $userId = null): Wopi { diff --git a/tests/features/bootstrap/WopiContext.php b/tests/features/bootstrap/WopiContext.php index 81215a270d..f333d8bbe9 100644 --- a/tests/features/bootstrap/WopiContext.php +++ b/tests/features/bootstrap/WopiContext.php @@ -31,6 +31,7 @@ use GuzzleHttp\Psr7\Utils; use JuliusHaertl\NextcloudBehat\Context\FilesContext; use JuliusHaertl\NextcloudBehat\Context\ServerContext; +use JuliusHaertl\NextcloudBehat\Context\SharingContext; use PHPUnit\Framework\Assert; class WopiContext implements Context { @@ -38,6 +39,8 @@ class WopiContext implements Context { private $serverContext; /** @var FilesContext */ private $filesContext; + /** @var SharingContext */ + private $sharingContext; private $downloadedFile; private $response; @@ -56,6 +59,7 @@ public function gatherContexts(BeforeScenarioScope $scope) { $environment = $scope->getEnvironment(); $this->serverContext = $environment->getContext(ServerContext::class); $this->filesContext = $environment->getContext(FilesContext::class); + $this->sharingContext = $environment->getContext(SharingContext::class); } public function getWopiEndpointBaseUrl() { @@ -105,6 +109,32 @@ public function collaboraPuts($source) { } } + /** + * @Then /^Create new document as guest with file name "([^"]*)"$/ + */ + public function createDocumentAsGuest(string $name) { + $client = new Client(); + $options = [ + 'body' => json_encode([ + 'directoryPath' => '/', + 'fileName' => $name, + 'mimeType' => 'application/vnd.oasis.opendocument.text', + 'shareToken' => $this->sharingContext->getLastShareData()['token'], + 'templateId' => 0, + ]), + 'headers' => [ + 'Content-Type' => 'application/json', + 'OCS-ApiRequest' => 'true' + ], + ]; + + try { + $this->response = $client->post($this->getWopiEndpointBaseUrl() . 'ocs/v2.php/apps/richdocuments/api/v1/file', $options); + } catch (\GuzzleHttp\Exception\ClientException $e) { + $this->response = $e->getResponse(); + } + } + /** * @Then /^the WOPI HTTP status code should be "([^"]*)"$/ * @param int $statusCode diff --git a/tests/features/wopi.feature b/tests/features/wopi.feature index 16d23a288a..522e1d2fd7 100644 --- a/tests/features/wopi.feature +++ b/tests/features/wopi.feature @@ -342,3 +342,19 @@ Feature: WOPI | UserFriendlyName | user2-displayname | And Collabora downloads the file And Collabora downloads the file and it is equal to "./../emptyTemplates/template.ods" + + Scenario: Save as guest user to owner root + Given as user "user1" + And User "user1" creates a folder "SharedFolder" + And as "user1" create a share with + | path | /SharedFolder | + | shareType | 3 | + And Updating last share with + | permissions | 31 | + And Create new document as guest with file name "some-guest-document.odt" + And as "user1" the file "/SharedFolder/some-guest-document.odt" exists + And a guest opens the file "some-guest-document.odt" of the shared link + And Collabora fetches checkFileInfo + And Collabora saves the content of "./../emptyTemplates/template.ods" as "/saved-as-guest-document.odt" + And as "user1" the file "/SharedFolder/saved-as-guest-document.odt" exists + And as "user1" the file "/saved-as-guest-document.odt" does not exist diff --git a/tests/stub.phpstub b/tests/stub.phpstub index 107cd2fde9..d8d0220fc9 100644 --- a/tests/stub.phpstub +++ b/tests/stub.phpstub @@ -7,6 +7,10 @@ class OC_User { public static function setIncognitoMode($status) {} } +class OC_Hook { + public static function emit($signalClass, $signalName, $params = []); +} + namespace OC\Hooks { interface Emitter { /**