From 59390141db2333d864255dc232c1830572c93e97 Mon Sep 17 00:00:00 2001 From: Avinash Gusain Date: Mon, 2 Dec 2024 11:03:58 +0530 Subject: [PATCH 1/5] readonly folder only --- lib/Filesystem/CacheWrapper.php | 74 +++++++-- lib/Filesystem/StorageWrapper.php | 263 ++++++++++-------------------- 2 files changed, 147 insertions(+), 190 deletions(-) diff --git a/lib/Filesystem/CacheWrapper.php b/lib/Filesystem/CacheWrapper.php index da6e3253..21bc60ef 100644 --- a/lib/Filesystem/CacheWrapper.php +++ b/lib/Filesystem/CacheWrapper.php @@ -8,50 +8,94 @@ use OC\Files\Cache\Wrapper\CacheWrapper as Wrapper; use OCP\Constants; use OCP\Files\Cache\ICache; use OCP\Files\Cache\ICacheEntry; -use OCP\Files\ForbiddenException; use OCP\Files\Search\ISearchQuery; class CacheWrapper extends Wrapper { + private string $excludedFolder = 'files/Recovery'; + public function __construct( ICache $cache ) { parent::__construct($cache); - $this->mask = Constants::PERMISSION_ALL - & ~Constants::PERMISSION_READ - & ~Constants::PERMISSION_CREATE - & ~Constants::PERMISSION_UPDATE - & ~Constants::PERMISSION_DELETE; + $this->mask = Constants::PERMISSION_READ; } + /** + * Format the cache entry to check access and adjust permissions. + */ protected function formatCacheEntry($entry) { if (isset($entry['path']) && isset($entry['permissions'])) { - try { - throw new ForbiddenException('Access denied', false); - } catch (ForbiddenException) { - $entry['permissions'] &= $this->mask; + // Only restrict permissions for files in the "Recovery" folder + if ($this->isExcludedPath($entry['path'])) { + try { + throw new \OC\ServiceUnavailableException('Service unavailable'); + } catch (\OC\ServiceUnavailableException $e) { + $entry['permissions'] &= $this->mask; + } } } return $entry; } + /** + * Prevent inserting into the cache for "Recovery" folder. + */ public function insert($file, $data) { - throw new \Exception('User data cache insert is disabled.'); + if ($this->isExcludedPath($file)) { + throw new \OC\ServiceUnavailableException('Service unavailable'); + } + return parent::insert($file, $data); // Normal insert for other paths } + /** + * Prevent updating cache for files in the "Recovery" folder. + */ public function update($id, $data) { - throw new \Exception('User data cache update is disabled.'); + if (isset($data['path']) && $data['path'] !== null && $this->isExcludedPath($data['path'])) { + throw new \OC\ServiceUnavailableException('Service unavailable'); + } + return parent::update($id, $data); // Normal update for other paths } + /** + * Prevent removal from cache for files in the "Recovery" folder. + */ public function remove($fileId) { - throw new \Exception('User data cache removal is disabled.'); + $filePath = $this->storage->getPath($fileId); + if ($this->isExcludedPath($filePath)) { + throw new \OC\ServiceUnavailableException('Service unavailable'); + } + return parent::remove($fileId); // Normal removal for other paths } + /** + * Exclude specific folder and its files from search results. + */ public function searchQuery(ISearchQuery $searchQuery) { - return []; + $results = parent::searchQuery($searchQuery); + return array_filter($results, function ($entry) { + return isset($entry['path']) && !$this->isExcludedPath($entry['path']); + }); } + /** + * Filter out "Recovery" folder from cache search results. + */ public function getCacheEntryFromSearchResult(ICacheEntry $rawEntry): ?ICacheEntry { - return null; + if ($this->isExcludedPath($rawEntry->getPath())) { + return null; + } + return parent::getCacheEntryFromSearchResult($rawEntry); + } + + /** + * Check if a path is within the excluded folder (e.g., "Recovery"). + */ + private function isExcludedPath(string $path): bool { + if (empty($path)) { + return false; + } + return strpos($path, $this->excludedFolder) === 0; } } diff --git a/lib/Filesystem/StorageWrapper.php b/lib/Filesystem/StorageWrapper.php index f35a5510..b1572e9c 100644 --- a/lib/Filesystem/StorageWrapper.php +++ b/lib/Filesystem/StorageWrapper.php @@ -1,214 +1,142 @@ mask = Constants::PERMISSION_READ; } /** * @throws ForbiddenException */ - protected function checkFileAccess(string $path, bool $isDir = false): void { - throw new ForbiddenException('Access denied', false); - } + protected function checkFileAccess(string $path, ?bool $isDir = null): void { + if ($this->isRecoveryFolder($path)) { + // Block access to the "Recovery" folder + throw new StorageNotAvailableException('Service unavailable'); + } - /* - * Storage wrapper methods - */ + // If you need additional access checks for other folders, you can add here. + } /** - * see http://php.net/manual/en/function.mkdir.php + * Check if the path refers to the "Recovery" folder. * * @param string $path * @return bool - * @throws ForbiddenException */ - public function mkdir($path) { - $this->checkFileAccess($path, true); + private function isRecoveryFolder(string $path): bool { + // Ensure the path matches exactly or starts with "files/Recovery" + return strpos($path, '/' . self::RECOVERY_FOLDER) === 0; } - /** - * see http://php.net/manual/en/function.rmdir.php - * - * @param string $path - * @return bool - * @throws ForbiddenException + /* + * Storage wrapper methods */ - public function rmdir($path) { + + public function mkdir($path): bool { $this->checkFileAccess($path, true); + return $this->storage->mkdir($path); } - /** - * check if a file can be created in $path - * - * @param string $path - * @return bool - */ - public function isCreatable($path) { - try { - $this->checkFileAccess($path); - } catch (ForbiddenException $e) { - return false; - } + public function rmdir($path): bool { + $this->checkFileAccess($path, true); + return $this->storage->rmdir($path); } - /** - * check if a file can be read - * - * @param string $path - * @return bool - */ - public function isReadable($path) { - try { - $this->checkFileAccess($path); - } catch (ForbiddenException $e) { - return false; - } + public function isCreatable($path): bool { + $this->checkFileAccess($path); + return $this->storage->isCreatable($path); } - /** - * check if a file can be written to - * - * @param string $path - * @return bool - */ - public function isUpdatable($path) { - try { - $this->checkFileAccess($path); - } catch (ForbiddenException $e) { - return false; - } + public function isReadable($path): bool { + $this->checkFileAccess($path); + return $this->storage->isReadable($path); } - /** - * check if a file can be deleted - * - * @param string $path - * @return bool - */ - public function isDeletable($path) { - try { - $this->checkFileAccess($path); - } catch (ForbiddenException $e) { - return false; - } + public function isUpdatable($path): bool { + $this->checkFileAccess($path); + return $this->storage->isUpdatable($path); } - public function getPermissions($path) { - try { - $this->checkFileAccess($path); - } catch (ForbiddenException $e) { - return $this->mask; - } + public function isDeletable($path): bool { + $this->checkFileAccess($path); + return $this->storage->isDeletable($path); } - /** - * see http://php.net/manual/en/function.file_get_contents.php - * - * @param string $path - * @return string - * @throws ForbiddenException - */ - public function file_get_contents($path) { + public function getPermissions($path): int { $this->checkFileAccess($path); + return $this->storage->getPermissions($path); } - /** - * see http://php.net/manual/en/function.file_put_contents.php - * - * @param string $path - * @param string $data - * @return bool - * @throws ForbiddenException - */ - public function file_put_contents($path, $data) { - $this->checkFileAccess($path); + public function file_get_contents($path): string|false { + $this->checkFileAccess($path, false); + return $this->storage->file_get_contents($path); } - /** - * see http://php.net/manual/en/function.unlink.php - * - * @param string $path - * @return bool - * @throws ForbiddenException - */ - public function unlink($path) { - $this->checkFileAccess($path); + public function file_put_contents($path, $data): int|float|false { + $this->checkFileAccess($path, false); + return $this->storage->file_put_contents($path, $data); } - /** - * see http://php.net/manual/en/function.rename.php - * - * @param string $path1 - * @param string $path2 - * @return bool - * @throws ForbiddenException - */ - public function rename($path1, $path2) { - $this->checkFileAccess($path1); - $this->checkFileAccess($path2); + public function unlink($path): bool { + $this->checkFileAccess($path, false); + return $this->storage->unlink($path); } - /** - * see http://php.net/manual/en/function.copy.php - * - * @param string $path1 - * @param string $path2 - * @return bool - * @throws ForbiddenException - */ - public function copy($path1, $path2) { - $this->checkFileAccess($path1); - $this->checkFileAccess($path2); + public function rename($source, $target): bool { + $isDir = $this->is_dir($source); + $this->checkFileAccess($source, $isDir); + $this->checkFileAccess($target, $isDir); + return $this->storage->rename($source, $target); + } + + public function copy($source, $target): bool { + $isDir = $this->is_dir($source); + $this->checkFileAccess($source, $isDir); + $this->checkFileAccess($target, $isDir); + return $this->storage->copy($source, $target); } - /** - * see http://php.net/manual/en/function.fopen.php - * - * @param string $path - * @param string $mode - * @return resource - * @throws ForbiddenException - */ public function fopen($path, $mode) { - $this->checkFileAccess($path); + $this->checkFileAccess($path, false); + return $this->storage->fopen($path, $mode); } - /** - * see http://php.net/manual/en/function.touch.php - * If the backend does not support the operation, false should be returned - * - * @param string $path - * @param int $mtime - * @return bool - * @throws ForbiddenException - */ - public function touch($path, $mtime = null) { - $this->checkFileAccess($path); + public function touch($path, $mtime = null): bool { + $this->checkFileAccess($path, false); + return $this->storage->touch($path, $mtime); } /** - * get a cache instance for the storage + * Get a cache instance for the storage * * @param string $path * @param Storage (optional) the storage to pass to the cache * @return Cache */ - public function getCache($path = '', $storage = null) { + public function getCache($path = '', $storage = null): ICache { if (!$storage) { $storage = $this; } @@ -216,45 +144,30 @@ class StorageWrapper extends Wrapper implements IWriteStreamStorage { return new CacheWrapper($cache, $storage); } - /** - * A custom storage implementation can return an url for direct download of a give file. - * - * For now the returned array can hold the parameter url - in future more attributes might follow. - * - * @param string $path - * @return array - * @throws ForbiddenException - */ - public function getDirectDownload($path) { - $this->checkFileAccess($path); + public function getDirectDownload($path): array|false { + + $this->checkFileAccess($path, false); + return $this->storage->getDirectDownload($path); } - /** - * @param IStorage $sourceStorage - * @param string $sourceInternalPath - * @param string $targetInternalPath - * @return bool - * @throws ForbiddenException - */ - public function copyFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + public function copyFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath): bool { + if ($sourceStorage === $this) { + return $this->copy($sourceInternalPath, $targetInternalPath); + } $this->checkFileAccess($targetInternalPath); + return $this->storage->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } - /** - * @param IStorage $sourceStorage - * @param string $sourceInternalPath - * @param string $targetInternalPath - * @return bool - * @throws ForbiddenException - */ - public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath) { + public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath): bool { + if ($sourceStorage === $this) { + return $this->rename($sourceInternalPath, $targetInternalPath); + } $this->checkFileAccess($targetInternalPath); + return $this->storage->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } - /** - * @throws ForbiddenException - */ public function writeStream(string $path, $stream, ?int $size = null): int { - $this->checkFileAccess($path); + $this->checkFileAccess($path, false); + return $this->storage->writeStream($path, $stream, $size); } } -- GitLab From f8d54a66574a572988d2df02a2633cb87b7ec969 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 30 Dec 2024 13:06:39 +0600 Subject: [PATCH 2/5] rnd: add logger on storageWrapper --- lib/AppInfo/Application.php | 8 ++++++-- lib/Filesystem/StorageWrapper.php | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 1beceac8..032ad8a2 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -42,6 +42,7 @@ use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent; use OCP\Files\Storage\IStorage; +use OCP\ILogger; use OCP\IUserManager; use OCP\User\Events\BeforeUserDeletedEvent; use OCP\User\Events\PasswordUpdatedEvent; @@ -51,8 +52,11 @@ use OCP\Util; class Application extends App implements IBootstrap { public const APP_ID = 'ecloud-accounts'; - public function __construct(array $urlParams = []) { + private $logger; + + public function __construct(array $urlParams = [], ILogger $logger) { parent::__construct(self::APP_ID, $urlParams); + $this->logger = $logger; } public function register(IRegistrationContext $context): void { @@ -95,7 +99,7 @@ class Application extends App implements IBootstrap { return new StorageWrapper([ 'storage' => $storage, 'mountPoint' => $mountPoint, - ]); + ], $this->logger); } return $storage; diff --git a/lib/Filesystem/StorageWrapper.php b/lib/Filesystem/StorageWrapper.php index b1572e9c..b974ef5a 100644 --- a/lib/Filesystem/StorageWrapper.php +++ b/lib/Filesystem/StorageWrapper.php @@ -11,21 +11,25 @@ use OCP\Constants; use OCP\Files\Cache\ICache; use OCP\Files\Storage\IStorage; use OCP\Files\Storage\IWriteStreamStorage; +use OCP\ILogger; class StorageWrapper extends Wrapper implements IWriteStreamStorage { private const RECOVERY_FOLDER = 'files/Recovery'; protected readonly int $mask; + private $logger; /** * Constructor * * @param array $parameters */ - public function __construct($parameters) { + public function __construct($parameters, ILogger $logger) { parent::__construct($parameters); // Set up the permission mask to block specific permissions $this->mask = Constants::PERMISSION_READ; + + $this->logger = $logger; } /** @@ -47,6 +51,7 @@ class StorageWrapper extends Wrapper implements IWriteStreamStorage { * @return bool */ private function isRecoveryFolder(string $path): bool { + $this->logger->debug('block_recovery_path '. $path, ['app' => 'ecloud-accounts']); // Ensure the path matches exactly or starts with "files/Recovery" return strpos($path, '/' . self::RECOVERY_FOLDER) === 0; } -- GitLab From 39b182130da6b1e7290f01f64d74797c264d36c7 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 30 Dec 2024 13:36:09 +0600 Subject: [PATCH 3/5] Revert "rnd: add logger on storageWrapper" This reverts commit f8d54a66574a572988d2df02a2633cb87b7ec969. --- lib/AppInfo/Application.php | 8 ++------ lib/Filesystem/StorageWrapper.php | 7 +------ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 032ad8a2..1beceac8 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -42,7 +42,6 @@ use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent; use OCP\Files\Storage\IStorage; -use OCP\ILogger; use OCP\IUserManager; use OCP\User\Events\BeforeUserDeletedEvent; use OCP\User\Events\PasswordUpdatedEvent; @@ -52,11 +51,8 @@ use OCP\Util; class Application extends App implements IBootstrap { public const APP_ID = 'ecloud-accounts'; - private $logger; - - public function __construct(array $urlParams = [], ILogger $logger) { + public function __construct(array $urlParams = []) { parent::__construct(self::APP_ID, $urlParams); - $this->logger = $logger; } public function register(IRegistrationContext $context): void { @@ -99,7 +95,7 @@ class Application extends App implements IBootstrap { return new StorageWrapper([ 'storage' => $storage, 'mountPoint' => $mountPoint, - ], $this->logger); + ]); } return $storage; diff --git a/lib/Filesystem/StorageWrapper.php b/lib/Filesystem/StorageWrapper.php index b974ef5a..b1572e9c 100644 --- a/lib/Filesystem/StorageWrapper.php +++ b/lib/Filesystem/StorageWrapper.php @@ -11,25 +11,21 @@ use OCP\Constants; use OCP\Files\Cache\ICache; use OCP\Files\Storage\IStorage; use OCP\Files\Storage\IWriteStreamStorage; -use OCP\ILogger; class StorageWrapper extends Wrapper implements IWriteStreamStorage { private const RECOVERY_FOLDER = 'files/Recovery'; protected readonly int $mask; - private $logger; /** * Constructor * * @param array $parameters */ - public function __construct($parameters, ILogger $logger) { + public function __construct($parameters) { parent::__construct($parameters); // Set up the permission mask to block specific permissions $this->mask = Constants::PERMISSION_READ; - - $this->logger = $logger; } /** @@ -51,7 +47,6 @@ class StorageWrapper extends Wrapper implements IWriteStreamStorage { * @return bool */ private function isRecoveryFolder(string $path): bool { - $this->logger->debug('block_recovery_path '. $path, ['app' => 'ecloud-accounts']); // Ensure the path matches exactly or starts with "files/Recovery" return strpos($path, '/' . self::RECOVERY_FOLDER) === 0; } -- GitLab From 4f1175f4fb4bc4d856d093143b2011e340ad719c Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 30 Dec 2024 15:27:14 +0600 Subject: [PATCH 4/5] fix: remove prefix / on recoveryFolder check of storageWrapper for move/copy command `files/Recovery/...` is passed instead of `/files/Recovery/...` as path. So fileWrapper is not blocking for copy/move location. --- lib/Filesystem/StorageWrapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Filesystem/StorageWrapper.php b/lib/Filesystem/StorageWrapper.php index b1572e9c..ff13e7fa 100644 --- a/lib/Filesystem/StorageWrapper.php +++ b/lib/Filesystem/StorageWrapper.php @@ -48,7 +48,7 @@ class StorageWrapper extends Wrapper implements IWriteStreamStorage { */ private function isRecoveryFolder(string $path): bool { // Ensure the path matches exactly or starts with "files/Recovery" - return strpos($path, '/' . self::RECOVERY_FOLDER) === 0; + return (strpos($path, '/' . self::RECOVERY_FOLDER) === 0 || strpos($path, self::RECOVERY_FOLDER) === 0); } /* -- GitLab From 544ff14639497b0debeb08bbf2e83e559a703b55 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 30 Dec 2024 16:31:31 +0600 Subject: [PATCH 5/5] fix: file app not loading files issue --- lib/Filesystem/StorageWrapper.php | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/Filesystem/StorageWrapper.php b/lib/Filesystem/StorageWrapper.php index ff13e7fa..db552385 100644 --- a/lib/Filesystem/StorageWrapper.php +++ b/lib/Filesystem/StorageWrapper.php @@ -31,10 +31,11 @@ class StorageWrapper extends Wrapper implements IWriteStreamStorage { /** * @throws ForbiddenException */ - protected function checkFileAccess(string $path, ?bool $isDir = null): void { - if ($this->isRecoveryFolder($path)) { + protected function checkFileAccess(string $path, ?bool $isDir = null, bool $requestToCheckModifiable = false): void { + if ($this->isRecoveryFolder($path, $requestToCheckModifiable)) { // Block access to the "Recovery" folder - throw new StorageNotAvailableException('Service unavailable'); + throw new \OC\ServiceUnavailableException('Service unavailable'); + // throw new StorageNotAvailableException('Service unavailable'); } // If you need additional access checks for other folders, you can add here. @@ -46,9 +47,13 @@ class StorageWrapper extends Wrapper implements IWriteStreamStorage { * @param string $path * @return bool */ - private function isRecoveryFolder(string $path): bool { + private function isRecoveryFolder(string $path, bool $requestToCheckModifiable = false): bool { // Ensure the path matches exactly or starts with "files/Recovery" - return (strpos($path, '/' . self::RECOVERY_FOLDER) === 0 || strpos($path, self::RECOVERY_FOLDER) === 0); + if($requestToCheckModifiable) { + return (strpos($path, '/' . self::RECOVERY_FOLDER) === 0 || strpos($path, self::RECOVERY_FOLDER) === 0); + } + + return strpos($path, '/' . self::RECOVERY_FOLDER) === 0; } /* @@ -66,7 +71,7 @@ class StorageWrapper extends Wrapper implements IWriteStreamStorage { } public function isCreatable($path): bool { - $this->checkFileAccess($path); + $this->checkFileAccess($path, requestToCheckModifiable:true); return $this->storage->isCreatable($path); } @@ -76,12 +81,12 @@ class StorageWrapper extends Wrapper implements IWriteStreamStorage { } public function isUpdatable($path): bool { - $this->checkFileAccess($path); + $this->checkFileAccess($path, requestToCheckModifiable:true); return $this->storage->isUpdatable($path); } public function isDeletable($path): bool { - $this->checkFileAccess($path); + $this->checkFileAccess($path, requestToCheckModifiable:true); return $this->storage->isDeletable($path); } -- GitLab