From f527b5103208de9b1de67cec3286b49087c1b94b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 10 Mar 2022 18:47:38 +0100 Subject: [PATCH] Move watermark logic to separate service and fix access of user for guests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl Refactor checkFileInfo response building Signed-off-by: Julius Härtl Add Watermark tests Signed-off-by: Julius Härtl Keep older php version supported Signed-off-by: Julius Härtl Stick to the actual user id when available Signed-off-by: Julius Härtl Make psalm happy again Signed-off-by: Julius Härtl --- composer/composer/autoload_classmap.php | 1 + composer/composer/autoload_static.php | 1 + lib/AppConfig.php | 2 +- lib/Controller/WopiController.php | 356 +++++++++------------ lib/Db/Wopi.php | 4 +- lib/Service/WatermarkService.php | 124 +++++++ lib/TemplateManager.php | 22 ++ tests/lib/AppConfigTest.php | 7 +- tests/lib/PermissionManagerTest.php | 33 ++ tests/lib/Service/WatermarkServiceTest.php | 180 +++++++++++ tests/phpunit.xml | 11 +- tests/psalm-baseline.xml | 12 - tests/stub.phpstub | 6 + 13 files changed, 534 insertions(+), 225 deletions(-) create mode 100644 lib/Service/WatermarkService.php create mode 100644 tests/lib/Service/WatermarkServiceTest.php diff --git a/composer/composer/autoload_classmap.php b/composer/composer/autoload_classmap.php index 6893c651e5..363c5680e0 100644 --- a/composer/composer/autoload_classmap.php +++ b/composer/composer/autoload_classmap.php @@ -73,6 +73,7 @@ 'OCA\\Richdocuments\\Service\\InitialStateService' => $baseDir . '/../lib/Service/InitialStateService.php', 'OCA\\Richdocuments\\Service\\RemoteService' => $baseDir . '/../lib/Service/RemoteService.php', 'OCA\\Richdocuments\\Service\\UserScopeService' => $baseDir . '/../lib/Service/UserScopeService.php', + 'OCA\\Richdocuments\\Service\\WatermarkService' => $baseDir . '/../lib/Service/WatermarkService.php', 'OCA\\Richdocuments\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php', 'OCA\\Richdocuments\\Settings\\Personal' => $baseDir . '/../lib/Settings/Personal.php', 'OCA\\Richdocuments\\Settings\\Section' => $baseDir . '/../lib/Settings/Section.php', diff --git a/composer/composer/autoload_static.php b/composer/composer/autoload_static.php index 83ea9ba251..0cc733241f 100644 --- a/composer/composer/autoload_static.php +++ b/composer/composer/autoload_static.php @@ -88,6 +88,7 @@ class ComposerStaticInitRichdocuments 'OCA\\Richdocuments\\Service\\InitialStateService' => __DIR__ . '/..' . '/../lib/Service/InitialStateService.php', 'OCA\\Richdocuments\\Service\\RemoteService' => __DIR__ . '/..' . '/../lib/Service/RemoteService.php', 'OCA\\Richdocuments\\Service\\UserScopeService' => __DIR__ . '/..' . '/../lib/Service/UserScopeService.php', + 'OCA\\Richdocuments\\Service\\WatermarkService' => __DIR__ . '/..' . '/../lib/Service/WatermarkService.php', 'OCA\\Richdocuments\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php', 'OCA\\Richdocuments\\Settings\\Personal' => __DIR__ . '/..' . '/../lib/Settings/Personal.php', 'OCA\\Richdocuments\\Settings\\Section' => __DIR__ . '/..' . '/../lib/Settings/Section.php', diff --git a/lib/AppConfig.php b/lib/AppConfig.php index 2cc826b84c..7d154c4bb3 100644 --- a/lib/AppConfig.php +++ b/lib/AppConfig.php @@ -88,7 +88,7 @@ public function getAppValue($key, $defaultValue = null) { public function getAppValueArray($key) { $value = $this->config->getAppValue($this->getAppNamespace($key), $key, []); if (array_key_exists($key, self::APP_SETTING_TYPES) && self::APP_SETTING_TYPES[$key] === 'array') { - $value = $value !== '' ? explode(',', $value) : []; + $value = !empty($value) ? explode(',', $value) : []; } return $value; } diff --git a/lib/Controller/WopiController.php b/lib/Controller/WopiController.php index 592ae6e290..c32bb1d05e 100644 --- a/lib/Controller/WopiController.php +++ b/lib/Controller/WopiController.php @@ -1,4 +1,6 @@ * @@ -21,6 +23,9 @@ namespace OCA\Richdocuments\Controller; +use Exception; +use OC\User\NoUserException; +use OCA\Encryption\Util as EncryptionUtil; use OCA\Files_Versions\Versions\IVersionManager; use OCA\Richdocuments\AppConfig; use OCA\Richdocuments\AppInfo\Application; @@ -33,19 +38,19 @@ use OCA\Richdocuments\PermissionManager; use OCA\Richdocuments\Service\FederationService; use OCA\Richdocuments\Service\UserScopeService; +use OCA\Richdocuments\Service\WatermarkService; use OCA\Richdocuments\TemplateManager; use OCA\Richdocuments\TokenManager; use OCP\AppFramework\Controller; -use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\StreamResponse; -use OCP\AppFramework\QueryException; use OCP\Constants; use OCP\Encryption\IManager as IEncryptionManager; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\File; -use OCP\Files\Folder; use OCP\Files\GenericFileException; use OCP\Files\InvalidPathException; use OCP\Files\IRootFolder; @@ -64,107 +69,56 @@ use OCP\IUserManager; use OCP\Lock\LockedException; use OCP\PreConditionNotMetException; +use OCP\Security\ISecureRandom; +use OCP\Server; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager as IShareManager; use OCP\Share\IShare; use Psr\Container\ContainerExceptionInterface; use Psr\Container\NotFoundExceptionInterface; use Psr\Log\LoggerInterface; +use RuntimeException; class WopiController extends Controller { - /** @var IRootFolder */ - private $rootFolder; - /** @var IURLGenerator */ - private $urlGenerator; - /** @var IConfig */ - private $config; - /** @var AppConfig */ - private $appConfig; - /** @var TokenManager */ - private $tokenManager; - /** @var PermissionManager */ - private $permissionManager; - /** @var IUserManager */ - private $userManager; - /** @var WopiMapper */ - private $wopiMapper; - /** @var LoggerInterface */ - private $logger; - /** @var TemplateManager */ - private $templateManager; - /** @var IShareManager */ - private $shareManager; - /** @var UserScopeService */ - private $userScopeService; - /** @var FederationService */ - private $federationService; - /** @var IEncryptionManager */ - private $encryptionManager; - /** @var IGroupManager */ - private $groupManager; - private ILockManager $lockManager; - private IEventDispatcher $eventDispatcher; - // Signifies LOOL that document has been changed externally in this storage public const LOOL_STATUS_DOC_CHANGED = 1010; public const WOPI_AVATAR_SIZE = 64; public function __construct( - $appName, + string $appName, IRequest $request, - IRootFolder $rootFolder, - IURLGenerator $urlGenerator, - IConfig $config, - AppConfig $appConfig, - TokenManager $tokenManager, - PermissionManager $permissionManager, - IUserManager $userManager, - WopiMapper $wopiMapper, - LoggerInterface $logger, - TemplateManager $templateManager, - IShareManager $shareManager, - UserScopeService $userScopeService, - FederationService $federationService, - IEncryptionManager $encryptionManager, - IGroupManager $groupManager, - ILockManager $lockManager, - IEventDispatcher $eventDispatcher + private IRootFolder $rootFolder, + private IURLGenerator $urlGenerator, + private IConfig $config, + private AppConfig $appConfig, + private TokenManager $tokenManager, + private PermissionManager $permissionManager, + private IUserManager $userManager, + private WopiMapper $wopiMapper, + private LoggerInterface $logger, + private TemplateManager $templateManager, + private IShareManager $shareManager, + private UserScopeService $userScopeService, + private FederationService $federationService, + private IEncryptionManager $encryptionManager, + private IGroupManager $groupManager, + private ILockManager $lockManager, + private IEventDispatcher $eventDispatcher, + private WatermarkService $watermarkService ) { parent::__construct($appName, $request); - $this->rootFolder = $rootFolder; - $this->urlGenerator = $urlGenerator; - $this->config = $config; - $this->appConfig = $appConfig; - $this->tokenManager = $tokenManager; - $this->permissionManager = $permissionManager; - $this->userManager = $userManager; - $this->wopiMapper = $wopiMapper; - $this->logger = $logger; - $this->templateManager = $templateManager; - $this->shareManager = $shareManager; - $this->userScopeService = $userScopeService; - $this->federationService = $federationService; - $this->encryptionManager = $encryptionManager; - $this->groupManager = $groupManager; - $this->lockManager = $lockManager; - $this->eventDispatcher = $eventDispatcher; } /** - * Returns general info about a file. - * - * @NoAdminRequired - * @NoCSRFRequired - * @PublicPage + * WOPI CheckFileInfo * - * @param string $fileId - * @param string $access_token - * @return JSONResponse - * @throws InvalidPathException - * @throws NotFoundException + * The CheckFileInfo operation returns information about a file, a user's permissions on that file, + * and general information about the capabilities that the WOPI host has on the file. */ - public function checkFileInfo($fileId, $access_token) { + #[NoCSRFRequired] + #[PublicPage] + public function checkFileInfo(string $fileId, string $access_token): JSONResponse { try { list($fileId, , $version) = Helper::parseFileId($fileId); @@ -187,13 +141,13 @@ public function checkFileInfo($fileId, $access_token) { } catch (ExpiredTokenException $e) { $this->logger->debug($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_UNAUTHORIZED); - } catch (\Exception $e) { + } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_FORBIDDEN); } $isPublic = empty($wopi->getEditorUid()); - $guestUserId = 'Guest-' . \OC::$server->getSecureRandom()->generate(8); + $guestUserId = 'Guest-' . Server::get(ISecureRandom::class)->generate(8); $user = $this->userManager->get($wopi->getEditorUid()); $userDisplayName = $user !== null && !$isPublic ? $user->getDisplayName() : $wopi->getGuestDisplayname(); $isVersion = $version !== '0'; @@ -210,7 +164,7 @@ public function checkFileInfo($fileId, $access_token) { 'BaseFileName' => $file->getName(), 'Size' => $file->getSize(), 'Version' => $version, - 'UserId' => !$isPublic ? $wopi->getEditorUid() : $guestUserId, + 'UserId' => !empty($wopi->getEditorUid()) ? $wopi->getEditorUid() : $guestUserId, 'OwnerId' => $wopi->getOwnerUid(), 'UserFriendlyName' => $userDisplayName, 'UserExtraInfo' => [], @@ -220,20 +174,23 @@ public function checkFileInfo($fileId, $access_token) { 'PostMessageOrigin' => $wopi->getServerHost(), 'LastModifiedTime' => Helper::toISO8601($file->getMTime()), 'SupportsRename' => !$isVersion, - 'UserCanRename' => !$isPublic && !$isVersion, + 'UserCanRename' => !$wopi->isGuest() && !$isVersion, 'EnableInsertRemoteImage' => !$isPublic, 'EnableShare' => $file->isShareable() && !$isVersion && !$isPublic, - 'HideUserList' => '', - 'DisablePrint' => $wopi->getHideDownload(), - 'DisableExport' => $wopi->getHideDownload(), - 'DisableCopy' => $wopi->getHideDownload(), - 'HideExportOption' => $wopi->getHideDownload(), - 'HidePrintOption' => $wopi->getHideDownload(), 'DownloadAsPostMessage' => $wopi->getDirect(), 'SupportsLocks' => $this->lockManager->isLockProviderAvailable(), 'IsUserLocked' => $this->permissionManager->userIsFeatureLocked($wopi->getEditorUid()), 'EnableRemoteLinkPicker' => (bool)$wopi->getCanwrite() && !$isPublic && !$wopi->getDirect(), + 'HideUserList' => '', ]; + $response = array_merge( + $response, + // TODO: Once PHP is >= 8.1 we can use array unpacking with string-keyed arrays + $this->templateManager->getWopiParams($wopi), + $this->watermarkService->getWopiParams($wopi), + $this->checkFileInfoUserExtraInfo($wopi), + $this->checkFileInfoHideDownload($wopi), + ); $enableZotero = $this->config->getAppValue(Application::APPNAME, 'zoteroEnabled', 'yes') === 'yes'; if (!$isPublic && $enableZotero) { @@ -251,22 +208,6 @@ public function checkFileInfo($fileId, $access_token) { $response['TemplateSaveAs'] = $file->getName(); } - $share = $this->getShareForWopiToken($wopi); - if ($this->permissionManager->shouldWatermark($file, $wopi->getEditorUid(), $share)) { - $email = $user !== null && !$isPublic ? $user->getEMailAddress() : ""; - $replacements = [ - 'userId' => $wopi->getEditorUid(), - 'date' => (new \DateTime())->format('Y-m-d H:i:s'), - 'themingName' => \OC::$server->getThemingDefaults()->getName(), - 'userDisplayName' => $userDisplayName, - 'email' => $email, - ]; - $watermarkTemplate = $this->appConfig->getAppValue('watermark_text'); - $response['WatermarkText'] = preg_replace_callback('/{(.+?)}/', function ($matches) use ($replacements) { - return $replacements[$matches[1]]; - }, $watermarkTemplate); - } - $user = $this->userManager->get($wopi->getEditorUid()); if ($user !== null) { $response['UserExtraInfo']['avatar'] = $this->urlGenerator->linkToRouteAbsolute('core.avatar.getAvatar', ['userId' => $wopi->getEditorUid(), 'size' => self::WOPI_AVATAR_SIZE]); @@ -288,17 +229,38 @@ public function checkFileInfo($fileId, $access_token) { $response = array_merge($response, $this->appConfig->getWopiOverride()); $this->eventDispatcher->dispatchTyped(new DocumentOpenedEvent( - $user ? $user->getUID() : null, + $user?->getUID(), $file )); return new JSONResponse($response); } + private function checkFileInfoUserExtraInfo(Wopi $wopi): array { + $editorUid = $wopi->getEditorUid(); + $user = $editorUid !== null ? $this->userManager->get($editorUid) : null; - private function setFederationFileInfo(Wopi $wopi, $response) { - $response['UserId'] = 'Guest-' . \OC::$server->getSecureRandom()->generate(8); + if($user === null || $editorUid === null) { + return [ + 'UserExtraInfo' => [ + 'avatar' => $this->urlGenerator->linkToRouteAbsolute('core.GuestAvatar.getAvatar', ['guestName' => urlencode($wopi->getGuestDisplayname()), 'size' => self::WOPI_AVATAR_SIZE]) + ] + ]; + } + + return [ + 'UserExtraInfo' => [ + 'avatar' => $this->urlGenerator->linkToRouteAbsolute('core.avatar.getAvatar', ['userId' => $editorUid, 'size' => self::WOPI_AVATAR_SIZE]), + 'is_admin' => $this->groupManager->isAdmin($editorUid), + ] + ]; + } + private function setFederationFileInfo(Wopi $wopi, array $response): array { + if (!$wopi->isRemoteToken()) { + return $response; + } + $response['UserId'] = 'Guest-' . \OC::$server->getSecureRandom()->generate(8); if ($wopi->getTokenType() === Wopi::TOKEN_TYPE_REMOTE_USER) { $remoteUserId = $wopi->getGuestDisplayname(); $cloudID = \OC::$server->getCloudIdManager()->resolveCloudId($remoteUserId); @@ -336,22 +298,23 @@ private function setFederationFileInfo(Wopi $wopi, $response) { return $response; } + private function checkFileInfoHideDownload(Wopi $wopi): array { + return [ + 'DisablePrint' => $wopi->getHideDownload(), + 'DisableExport' => $wopi->getHideDownload(), + 'DisableCopy' => $wopi->getHideDownload(), + 'HideExportOption' => $wopi->getHideDownload(), + 'HidePrintOption' => $wopi->getHideDownload(), + ]; + } + /** - * Given an access token and a fileId, returns the contents of the file. - * Expects a valid token in access_token parameter. - * - * @PublicPage - * @NoCSRFRequired - * - * @param string $fileId - * @param string $access_token - * @return Http\Response - * @throws NotFoundException - * @throws NotPermittedException - * @throws LockedException + * WOPI GetFile + * The GetFile operation retrieves a file from a host. */ - public function getFile($fileId, - $access_token) { + #[PublicPage] + #[NoCSRFRequired] + public function getFile(string $fileId, string $access_token): JSONResponse|Http\Response { list($fileId, , $version) = Helper::parseFileId($fileId); try { @@ -362,7 +325,7 @@ public function getFile($fileId, } catch (ExpiredTokenException $e) { $this->logger->debug($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_UNAUTHORIZED); - } catch (\Exception $e) { + } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_FORBIDDEN); } @@ -375,7 +338,12 @@ public function getFile($fileId, if ($wopi->isTemplateToken()) { $this->templateManager->setUserId($wopi->getOwnerUid()); $file = $this->templateManager->get($wopi->getFileid()); - $response = new StreamResponse($file->fopen('rb')); + $handler = $file->fopen('rb'); + if (!$handler) { + $this->logger->error('getFile failed to retrurn template file'); + return new JSONResponse([], Http::STATUS_FORBIDDEN); + } + $response = new StreamResponse($handler); $response->addHeader('Content-Disposition', 'attachment'); $response->addHeader('Content-Type', 'application/octet-stream'); return $response; @@ -403,7 +371,7 @@ public function getFile($fileId, $response->addHeader('Content-Disposition', 'attachment'); $response->addHeader('Content-Type', 'application/octet-stream'); return $response; - } catch (\Exception $e) { + } catch (Exception $e) { $this->logger->error('getFile failed: ' . $e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_FORBIDDEN); } catch (NotFoundExceptionInterface|ContainerExceptionInterface $e) { @@ -413,18 +381,12 @@ public function getFile($fileId, } /** - * Given an access token and a fileId, replaces the files with the request body. - * Expects a valid token in access_token parameter. - * - * @PublicPage - * @NoCSRFRequired - * - * @param string $fileId - * @param string $access_token - * @return JSONResponse + * WOPI PutFile + * The PutFile operation updates a file’s binary contents. */ - public function putFile($fileId, - $access_token) { + #[PublicPage] + #[NoCSRFRequired] + public function putFile(string $fileId, string $access_token): JSONResponse { list($fileId, , ) = Helper::parseFileId($fileId); $isPutRelative = ($this->request->getHeader('X-WOPI-Override') === 'PUT_RELATIVE'); @@ -436,7 +398,7 @@ public function putFile($fileId, } catch (ExpiredTokenException $e) { $this->logger->debug($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_UNAUTHORIZED); - } catch (\Exception $e) { + } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_FORBIDDEN); } @@ -495,21 +457,25 @@ public function putFile($fileId, $file = $this->getFileForWopiToken($wopi); $wopiHeaderTime = $this->request->getHeader('X-LOOL-WOPI-Timestamp'); - if (!empty($wopiHeaderTime) && $wopiHeaderTime !== Helper::toISO8601($file->getMTime() ?? 0)) { + if (!empty($wopiHeaderTime) && $wopiHeaderTime !== Helper::toISO8601($file->getMTime())) { $this->logger->debug('Document timestamp mismatch ! WOPI client says mtime {headerTime} but storage says {storageTime}', [ 'headerTime' => $wopiHeaderTime, - 'storageTime' => Helper::toISO8601($file->getMTime() ?? 0) + 'storageTime' => Helper::toISO8601($file->getMTime()) ]); // Tell WOPI client about this conflict. return new JSONResponse(['LOOLStatusCode' => self::LOOL_STATUS_DOC_CHANGED], Http::STATUS_CONFLICT); } } + if (!$file instanceof File) { + throw new Exception('Unexpected node type'); + } + $content = fopen('php://input', 'rb'); try { - $this->wrappedFilesystemOperation($wopi, function () use ($file, $content) { - return $file->putContent($content); + $this->wrappedFilesystemOperation($wopi, function () use ($file, $content): void { + $file->putContent($content); }); } catch (LockedException $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); @@ -533,7 +499,7 @@ public function putFile($fileId, } catch (NotFoundException $e) { $this->logger->warning($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_NOT_FOUND); - } catch (\Exception $e) { + } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); } @@ -546,15 +512,9 @@ public function putFile($fileId, * handles both saving and saving as.* Given an access token and a fileId, replaces the files with the request body. * * FIXME Cleanup this code as is a lot of shared logic between putFile and putRelativeFile - * - * @PublicPage - * @NoCSRFRequired - * - * @param string $fileId - * @param string $access_token - * @return JSONResponse - * @throws DoesNotExistException */ + #[PublicPage] + #[NoCSRFRequired] public function postFile(string $fileId, string $access_token): JSONResponse { try { $wopiOverride = $this->request->getHeader('X-WOPI-Override'); @@ -570,7 +530,7 @@ public function postFile(string $fileId, string $access_token): JSONResponse { } catch (ExpiredTokenException $e) { $this->logger->debug($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_UNAUTHORIZED); - } catch (\Exception $e) { + } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_FORBIDDEN); } @@ -626,13 +586,6 @@ public function postFile(string $fileId, string $access_token): JSONResponse { $path = $userFolder->getPath() . $suggested; } - if ($path === '') { - return new JSONResponse([ - 'status' => 'error', - 'message' => 'Cannot rename the file' - ]); - } - // create the folder first if (!$this->rootFolder->nodeExists(dirname($path))) { $this->rootFolder->newFolder(dirname($path)); @@ -672,14 +625,18 @@ public function postFile(string $fileId, string $access_token): JSONResponse { $file = $this->rootFolder->newFile($path); } + if (!$file instanceof File) { + throw new Exception('Unexpected node type'); + } + $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()); try { - $this->wrappedFilesystemOperation($wopi, function () use ($file, $content) { - return $file->putContent($content); + $this->wrappedFilesystemOperation($wopi, function () use ($file, $content): void { + $file->putContent($content); }); } catch (LockedException $e) { return new JSONResponse(['message' => 'File locked'], Http::STATUS_INTERNAL_SERVER_ERROR); @@ -701,7 +658,7 @@ public function postFile(string $fileId, string $access_token): JSONResponse { } catch (NotFoundException $e) { $this->logger->warning($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_NOT_FOUND); - } catch (\Exception $e) { + } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); } @@ -715,7 +672,7 @@ private function lock(Wopi $wopi, string $lock): JSONResponse { Application::APPNAME )); return new JSONResponse(); - } catch (NoLockProviderException|PreConditionNotMetException $e) { + } catch (NoLockProviderException|PreConditionNotMetException) { return new JSONResponse([], Http::STATUS_BAD_REQUEST); } catch (OwnerLockedException $e) { // If a file is manually locked by a user we want to all this user to still perform a WOPI lock and write @@ -724,7 +681,7 @@ private function lock(Wopi $wopi, string $lock): JSONResponse { } return new JSONResponse([], Http::STATUS_LOCKED); - } catch (\Exception $e) { + } catch (Exception $e) { return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); } } @@ -743,7 +700,7 @@ private function unlock(Wopi $wopi, string $lock): JSONResponse { return new JSONResponse(); } return new JSONResponse([], Http::STATUS_BAD_REQUEST); - } catch (\Exception $e) { + } catch (Exception $e) { return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); } } @@ -760,7 +717,7 @@ private function refreshLock(Wopi $wopi, string $lock): JSONResponse { return new JSONResponse([], Http::STATUS_BAD_REQUEST); } catch (OwnerLockedException $e) { return new JSONResponse([], Http::STATUS_LOCKED); - } catch (\Exception $e) { + } catch (Exception $e) { return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); } } @@ -777,7 +734,7 @@ private function getLock(Wopi $wopi, string $lock): JSONResponse { * @throws ShareNotFound */ protected function wrappedFilesystemOperation(Wopi $wopi, callable $filesystemOperation): void { - $retryOperation = function () use ($filesystemOperation) { + $retryOperation = function () use ($filesystemOperation): void { $this->retryOperation($filesystemOperation); }; try { @@ -798,7 +755,7 @@ protected function wrappedFilesystemOperation(Wopi $wopi, callable $filesystemOp * @throws LockedException * @throws GenericFileException */ - private function retryOperation(callable $operation) { + private function retryOperation(callable $operation): void { for ($i = 0; $i < 5; $i++) { try { if ($operation() !== false) { @@ -815,12 +772,13 @@ private function retryOperation(callable $operation) { } /** - * @param Wopi $wopi - * @return File|Folder|Node|null * @throws NotFoundException * @throws ShareNotFound + * @throws InvalidPathException + * @throws NotPermittedException + * @throws NoUserException */ - private function getFileForWopiToken(Wopi $wopi) { + private function getFileForWopiToken(Wopi $wopi): File { if (!empty($wopi->getShare())) { $share = $this->shareManager->getShareByToken($wopi->getShare()); $node = $share->getNode(); @@ -830,7 +788,12 @@ private function getFileForWopiToken(Wopi $wopi) { } $nodes = $node->getById($wopi->getFileid()); - return array_shift($nodes); + + $file = array_shift($nodes); + if (!$file instanceof File) { + throw new NotFoundException('Unexpected node type'); + } + return $file; } // Group folders requires an active user to be set in order to apply the proper acl permissions as for anonymous requests it requires share permissions for read access @@ -851,7 +814,11 @@ private function getFileForWopiToken(Wopi $wopi) { return ($b->getPermissions() & Constants::PERMISSION_UPDATE) <=> ($a->getPermissions() & Constants::PERMISSION_UPDATE); }); - return array_shift($files); + $file = array_shift($files); + if (!$file instanceof File) { + throw new NotFoundException('Unexpected node type'); + } + return $file; } private function getShareForWopiToken(Wopi $wopi): ?IShare { @@ -865,35 +832,32 @@ private function getShareForWopiToken(Wopi $wopi): ?IShare { /** * Endpoint to return the template file that is requested by collabora to create a new document - * - * @PublicPage - * @NoCSRFRequired - * - * @param $fileId - * @param $access_token - * @return JSONResponse|StreamResponse */ - public function getTemplate($fileId, $access_token) { + #[PublicPage] + #[NoCSRFRequired] + public function getTemplate(string $fileId, string $access_token): JSONResponse|StreamResponse { try { $wopi = $this->wopiMapper->getWopiForToken($access_token); - } catch (UnknownTokenException $e) { - return new JSONResponse([], Http::STATUS_FORBIDDEN); - } catch (ExpiredTokenException $e) { - return new JSONResponse([], Http::STATUS_UNAUTHORIZED); - } - if ((int)$fileId !== $wopi->getTemplateId()) { - return new JSONResponse([], Http::STATUS_FORBIDDEN); - } + if ((int)$fileId !== $wopi->getTemplateId()) { + return new JSONResponse([], Http::STATUS_FORBIDDEN); + } - try { $this->templateManager->setUserId($wopi->getOwnerUid()); $file = $this->templateManager->get($wopi->getTemplateId()); - $response = new StreamResponse($file->fopen('rb')); + $handler = $file->fopen('rb'); + if (!$handler) { + throw new RuntimeException('getTemplate failed to return template file'); + } + $response = new StreamResponse($handler); $response->addHeader('Content-Disposition', 'attachment'); $response->addHeader('Content-Type', 'application/octet-stream'); return $response; - } catch (\Exception $e) { + } catch (UnknownTokenException $e) { + return new JSONResponse([], Http::STATUS_FORBIDDEN); + } catch (ExpiredTokenException $e) { + return new JSONResponse([], Http::STATUS_UNAUTHORIZED); + } catch (Exception $e) { $this->logger->error('getTemplate failed: ' . $e->getMessage(), ['exception' => $e]); return new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); } @@ -904,9 +868,9 @@ public function getTemplate($fileId, $access_token) { */ private function isMasterKeyEnabled(): bool { try { - $util = \OC::$server->query(\OCA\Encryption\Util::class); + $util = Server::get(EncryptionUtil::class); return $util->isMasterKeyEnabled(); - } catch (QueryException $e) { + } catch (NotFoundExceptionInterface|ContainerExceptionInterface) { // No encryption module enabled return false; } diff --git a/lib/Db/Wopi.php b/lib/Db/Wopi.php index 2e14767aff..28940c7fc1 100644 --- a/lib/Db/Wopi.php +++ b/lib/Db/Wopi.php @@ -30,7 +30,7 @@ * @method void setOwnerUid(string $uid) * @method string getOwnerUid() * @method void setEditorUid(?string $uid) - * @method string getEditorUid() + * @method string|null getEditorUid() * @method void setFileid(int $fileid) * @method int getFileid() * @method void setVersion(int $version) @@ -53,7 +53,7 @@ * @method string getGuestDisplayname() * @method void setTemplateDestination(int $fileId) * @method int getTemplateDestination() - * @method void setTemplateId(int $fileId) + * @method void setTemplateId(?int $fileId) * @method int getTemplateId() * @method void setShare(string $token) * @method string getShare() diff --git a/lib/Service/WatermarkService.php b/lib/Service/WatermarkService.php new file mode 100644 index 0000000000..a4d0f577b7 --- /dev/null +++ b/lib/Service/WatermarkService.php @@ -0,0 +1,124 @@ +config = $config; + $this->appConfig = $appConfig; + $this->groupManager = $groupManager; + $this->systemTagObjectMapper = $systemTagObjectMapper; + $this->rootFolder = $rootFolder; + } + + public function getWopiParams(Wopi $wopi): array { + if ($this->shouldWatermark($wopi)) { + return [ + 'WatermarkText' => $this->getWatermarkText($wopi) + ]; + } + + return []; + } + + public function shouldWatermark(Wopi $wopi): bool { + $userId = $wopi->getEditorUid(); + $fileId = $wopi->getFileid(); + $isPublic = $wopi->isGuest(); + if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_enabled', 'no') === 'no') { + return false; + } + + if ($isPublic) { + if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkAll', 'no') === 'yes') { + return true; + } + if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkRead', 'no') === 'yes' && !$wopi->getCanwrite()) { + return true; + } + if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkSecure', 'no') === 'yes' && $wopi->getHideDownload()) { + return true; + } + if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_linkTags', 'no') === 'yes') { + $tags = $this->appConfig->getAppValueArray('watermark_linkTagsList'); + $fileTags = $this->systemTagObjectMapper->getTagIdsForObjects([$fileId], 'files')[$fileId]; + foreach ($fileTags as $tagId) { + if (in_array($tagId, $tags, true)) { + return true; + } + } + } + } else { + if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_shareAll', 'no') === 'yes') { + // Fallback to the file owner for watermarking check for public re-shares + $userId = $userId ?? $wopi->getOwnerUid(); + $files = $this->rootFolder->getUserFolder($wopi->getOwnerUid())->getById($fileId); + if (count($files) !== 0 && $files[0]->getOwner()->getUID() !== $userId) { + return true; + } + } + if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_shareRead', 'no') === 'yes' && !$wopi->getCanwrite()) { + return true; + } + } + if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_allGroups', 'no') === 'yes') { + // Fallback to the file owner for watermarking check for public shares + $userId = $userId ?? $wopi->getOwnerUid(); + $groups = $this->appConfig->getAppValueArray('watermark_allGroupsList'); + + foreach ($groups as $group) { + if ($this->groupManager->isInGroup($userId, $group)) { + return true; + } + } + } + if ($this->config->getAppValue(AppConfig::WATERMARK_APP_NAMESPACE, 'watermark_allTags', 'no') === 'yes') { + $tags = $this->appConfig->getAppValueArray('watermark_allTagsList'); + $fileTags = $this->systemTagObjectMapper->getTagIdsForObjects([$fileId], 'files')[$fileId]; + foreach ($fileTags as $tagId) { + if (in_array($tagId, $tags, true)) { + return true; + } + } + } + + return false; + } + + public function getWatermarkText(Wopi $wopi) { + $user = \OC::$server->getUserManager()->get($wopi->getEditorUid()); + $email = $user !== null && !$wopi->isGuest() ? $user->getEMailAddress() : ""; + $replacements = [ + 'userId' => $wopi->getEditorUid(), + 'date' => (new \DateTime())->format('Y-m-d H:i:s'), + 'themingName' => \OC::$server->getThemingDefaults()->getName(), + 'userDisplayName' => $user ? $user->getDisplayName() : $wopi->getGuestDisplayname(), + 'email' => $email, + ]; + $watermarkTemplate = $this->appConfig->getAppValue('watermark_text'); + return preg_replace_callback('/{(.+?)}/', function ($matches) use ($replacements) { + return $replacements[$matches[1]]; + }, $watermarkTemplate); + } + +} diff --git a/lib/TemplateManager.php b/lib/TemplateManager.php index 823c41ceb0..8347d0c422 100644 --- a/lib/TemplateManager.php +++ b/lib/TemplateManager.php @@ -26,6 +26,7 @@ namespace OCA\Richdocuments; use OCA\Richdocuments\AppInfo\Application; +use OCA\Richdocuments\Db\Wopi; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\File; use OCP\Files\Folder; @@ -603,4 +604,25 @@ public function getTemplateSource(int $fileId): ?File { return null; } + + public function getWopiParams(Wopi $wopi): array { + if ($wopi->hasTemplateId()) { + $templateUrl = 'index.php/apps/richdocuments/wopi/template/' . $wopi->getTemplateId() . '?access_token=' . $wopi->getToken(); + $templateUrl = $this->urlGenerator->getAbsoluteURL($templateUrl); + return [ + 'TemplateSource' => $templateUrl + ]; + } + + if ($wopi->isTemplateToken()) { + // FIXME: Remove backward compatibility layer once TemplateSource is available in all supported Collabora versions + $userFolder = $this->rootFolder->getUserFolder($wopi->getOwnerUid()); + $file = $userFolder->getById($wopi->getTemplateDestination())[0]; + return [ + 'TemplateSaveAs' => $file->getName() + ]; + } + + return []; + } } diff --git a/tests/lib/AppConfigTest.php b/tests/lib/AppConfigTest.php index 5ae885babc..ed56e7afc8 100644 --- a/tests/lib/AppConfigTest.php +++ b/tests/lib/AppConfigTest.php @@ -30,10 +30,9 @@ use Test\TestCase; class AppConfigTest extends TestCase { - /** @var IConfig|MockObject */ - private $config; - /** @var AppConfig */ - private $appConfig; + private IConfig|MockObject $config; + private IConfig|MockObject $appManager; + private AppConfig $appConfig; public function setUp(): void { parent::setUp(); diff --git a/tests/lib/PermissionManagerTest.php b/tests/lib/PermissionManagerTest.php index 357ebcb2cd..4ce9367a07 100644 --- a/tests/lib/PermissionManagerTest.php +++ b/tests/lib/PermissionManagerTest.php @@ -84,6 +84,30 @@ public static function dataGroupMatchGroups(): array { [[], [], true], ]; } + + public function testIsEnabledForUserEnabledNotInGroup() { + /** @var IUser|MockBuilder $user */ + $user = $this->createMock(IUser::class); + + $this->appConfig + ->expects($this->once()) + ->method('getUseGroups') + ->willReturn(['Enabled1', 'Enabled2', 'Enabled3']); + + $this->userManager + ->expects($this->once()) + ->method('get') + ->willReturn($user); + + $this->groupManager + ->expects(self::any()) + ->method('getUserGroupIds') + ->willReturnCallback(function ($user) { + return []; + }); + + $this->assertFalse($this->permissionManager->isEnabledForUser('TestUser')); + } /** @dataProvider dataGroupMatchGroups */ public function testEditGroups($editGroups, $userGroups, $result): void { @@ -132,6 +156,15 @@ public function testFeatureLock($editGroups, $userGroups, $result): void { $this->groupManager->expects($this->any()) ->method('getUserGroupIds') ->willReturn($userGroups); + $this->groupManager + ->expects(self::any()) + ->method('isInGroup') + ->willReturnCallback(function ($user, $group) { + if ($user === 'TestUser' && $group === 'Enabled2') { + return true; + } + return false; + }); $canEdit = $this->permissionManager->userCanEdit('user1'); $isLocked = $this->permissionManager->userIsFeatureLocked('user1'); diff --git a/tests/lib/Service/WatermarkServiceTest.php b/tests/lib/Service/WatermarkServiceTest.php new file mode 100644 index 0000000000..c66a5d4475 --- /dev/null +++ b/tests/lib/Service/WatermarkServiceTest.php @@ -0,0 +1,180 @@ +config = $this->createMock(IConfig::class); + $this->groupManager = $this->createMock(IGroupManager::class); + } + + public static function dataWatermark(): Generator { + yield [ + self::WATERMARK_USER, + self::WATERMARK_USER_OWNER, + [], + false + ]; + + yield [ + self::WATERMARK_USER, + self::WATERMARK_USER_OWNER, + ['watermark_enabled' => true], + false + ]; + + yield [ + self::WATERMARK_USER, + self::WATERMARK_USER_OWNER, + ['watermark_enabled' => true, 'watermark_linkAll' => true], + false + ]; + + yield [ + self::WATERMARK_USER, + self::WATERMARK_USER_RECIPIENT, + ['watermark_enabled' => true, 'watermark_shareAll' => true], + true + ]; + + yield [ + self::WATERMARK_USER, + self::WATERMARK_USER_RECIPIENT, + ['watermark_enabled' => true, 'watermark_allGroups' => true, 'watermark_allGroupsList' => ''], + false + ]; + + yield [ + self::WATERMARK_USER, + self::WATERMARK_USER_RECIPIENT, + ['watermark_enabled' => true, 'watermark_allGroups' => true, 'watermark_allGroupsList' => 'group2,group3'], + false + ]; + + yield [ + self::WATERMARK_USER, + self::WATERMARK_USER_RECIPIENT, + ['watermark_enabled' => true, 'watermark_allGroups' => true, 'watermark_allGroupsList' => 'group2,group3,group3,group1'], + true + ]; + + yield [ + self::WATERMARK_USER, + self::WATERMARK_USER_RECIPIENT, + ['watermark_enabled' => true, 'watermark_allGroups' => true, 'watermark_allGroupsList' => 'group1'], + true + ]; + + yield [ + self::WATERMARK_PUBLIC, + self::WATERMARK_USER_GUEST, + [], + false + ]; + + yield [ + self::WATERMARK_PUBLIC, + self::WATERMARK_USER_GUEST, + ['watermark_enabled' => true], + false + ]; + + yield [ + self::WATERMARK_PUBLIC, + self::WATERMARK_USER_GUEST, + ['watermark_enabled' => true, 'watermark_linkAll' => true], + true + ]; + + yield [ + self::WATERMARK_PUBLIC, + self::WATERMARK_USER_GUEST, + ['watermark_enabled' => true, 'watermark_linkAll' => true], + true + ]; + } + + private function createWatermarkConfigMock(array $config): void { + $this->config->expects(self::any()) + ->method('getAppValue') + ->willReturnCallback(function ($app, $key, $default) use ($config) { + if (isset($config[$key])) { + if (is_bool($config[$key])) { + return $config[$key] ? 'yes' : 'no'; + } + + return $config[$key]; + } + return $default; + }); + } + + /** @dataProvider dataWatermark */ + public function testWatermark(bool $isPublic, ?string $user, array $config, bool $expected): void { + $rootFolder = $this->createMock(IRootFolder::class); + $this->createWatermarkConfigMock($config); + $this->groupManager->expects(self::any()) + ->method('isInGroup') + ->willReturnCallback(function ($userId, $groupId) { + return ($userId === 'user1' && $groupId === 'group1'); + }); + + $controller = new WatermarkService( + $this->config, + new AppConfig($this->config, $this->createMock(IAppManager::class), $this->createMock(\OCP\GlobalScale\IConfig::class)), + $this->groupManager, + $this->createMock(ISystemTagObjectMapper::class), + $rootFolder + ); + + $fileId = 1234; + $userFolder = $this->createMock(Folder::class); + $file = $this->createMock(File::class); + $storageUser = $this->createMock(IUser::class); + $storageUser->method('getUID')->willReturn(self::WATERMARK_USER_OWNER); + $rootFolder->method('getUserFolder') + ->willReturn($userFolder); + $userFolder->method('getById')->willReturn([$file]); + $file->method('getOwner')->willReturn($storageUser); + + $wopi = new Wopi(); + $wopi->setFileid($fileId); + $wopi->setOwnerUid(self::WATERMARK_USER_OWNER); + $wopi->setEditorUid($user); + $wopi->setTokenType($isPublic ? Wopi::TOKEN_TYPE_GUEST : Wopi::TOKEN_TYPE_USER); + + self::assertEquals($expected, $this->invokePrivate($controller, 'shouldWatermark', [$wopi])); + } +} diff --git a/tests/phpunit.xml b/tests/phpunit.xml index 9b99baf28d..50cb1fdbd0 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -1,5 +1,5 @@ - + @@ -12,13 +12,4 @@ - - - ../../richdocuments - - - ../../richdocuments/l10n - ../../richdocuments/tests - - diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 7ed92ff27d..65b168b214 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -73,18 +73,6 @@ NotFoundResponse - - - null - - - - - - putContent - putContent - - $time diff --git a/tests/stub.phpstub b/tests/stub.phpstub index 9dda632f24..f897c6eb2f 100644 --- a/tests/stub.phpstub +++ b/tests/stub.phpstub @@ -98,3 +98,9 @@ class OC_Util { public static function setupFS(?string $user); public static function tearDownFS(); } + +namespace OCA\Encryption { + abstract class Util { + public function isMasterKeyEnabled(): bool; + } +}