From 5172baaf8b0b028328a59e7692bd395b5da2883b Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Fri, 17 Nov 2023 08:43:22 +0100 Subject: [PATCH] fix(ObjectStore): Make copying behavior consistent with local storage Drop file permissions on copy like we do on local storage. Signed-off-by: Ferdinand Thiessen --- .../Files/ObjectStore/ObjectStoreStorage.php | 7 ++++ lib/public/Files/Cache/ICacheEntry.php | 4 +- .../ObjectStore/ObjectStoreStorageTest.php | 38 +++++++++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php index 4dceee9a58bbf..eb8aaffe1e03e 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php +++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php @@ -68,6 +68,8 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFil private $logger; + private bool $handleCopiesAsOwned; + /** @var bool */ protected $validateWrites = true; @@ -88,6 +90,7 @@ public function __construct($params) { if (isset($params['validateWrites'])) { $this->validateWrites = (bool)$params['validateWrites']; } + $this->handleCopiesAsOwned = (bool)($params['handleCopiesAsOwned'] ?? false); $this->logger = \OC::$server->getLogger(); } @@ -651,6 +654,10 @@ private function copyFile(ICacheEntry $sourceEntry, string $to) { try { $this->objectStore->copyObject($sourceUrn, $targetUrn); + if ($this->handleCopiesAsOwned) { + // Copied the file thus we gain all permissions as we are the owner now ! warning while this aligns with local storage it should not be used and instead fix local storage ! + $cache->update($targetId, ['permissions' => \OCP\Constants::PERMISSION_ALL]); + } } catch (\Exception $e) { $cache->remove($to); diff --git a/lib/public/Files/Cache/ICacheEntry.php b/lib/public/Files/Cache/ICacheEntry.php index e1e8129394c71..3a069ca69e047 100644 --- a/lib/public/Files/Cache/ICacheEntry.php +++ b/lib/public/Files/Cache/ICacheEntry.php @@ -123,8 +123,8 @@ public function getStorageMTime(); public function getEtag(); /** - * Get the permissions for the file stored as bitwise combination of \OCP\PERMISSION_READ, \OCP\PERMISSION_CREATE - * \OCP\PERMISSION_UPDATE, \OCP\PERMISSION_DELETE and \OCP\PERMISSION_SHARE + * Get the permissions for the file stored as bitwise combination of \OCP\Constants::PERMISSION_READ, \OCP\Constants::PERMISSION_CREATE + * \OCP\Constants::PERMISSION_UPDATE, \OCP\Constants::PERMISSION_DELETE and \OCP\Constants::PERMISSION_SHARE * * @return int * @since 9.0.0 diff --git a/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php b/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php index 1bebaf6c4ba7b..2f83574707716 100644 --- a/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php +++ b/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php @@ -237,4 +237,42 @@ public function testCopyBetweenJails() { $this->assertEquals('2', $this->instance->file_get_contents('b/target/sub/2.txt')); $this->assertEquals('3', $this->instance->file_get_contents('b/target/sub/3.txt')); } + + public function testCopyPreservesPermissions() { + $cache = $this->instance->getCache(); + + $this->instance->file_put_contents('test.txt', 'foo'); + $this->assertTrue($cache->inCache('test.txt')); + + $cache->update($cache->getId('test.txt'), ['permissions' => \OCP\Constants::PERMISSION_READ]); + $this->assertEquals(\OCP\Constants::PERMISSION_READ, $this->instance->getPermissions('test.txt')); + + $this->assertTrue($this->instance->copy('test.txt', 'new.txt')); + + $this->assertTrue($cache->inCache('new.txt')); + $this->assertEquals(\OCP\Constants::PERMISSION_READ, $this->instance->getPermissions('new.txt')); + } + + /** + * Test that copying files will drop permissions like local storage does + * TODO: Drop this and fix local storage + */ + public function testCopyGrantsPermissions() { + $config['objectstore'] = $this->objectStorage; + $config['handleCopiesAsOwned'] = true; + $instance = new ObjectStoreStorageOverwrite($config); + + $cache = $instance->getCache(); + + $instance->file_put_contents('test.txt', 'foo'); + $this->assertTrue($cache->inCache('test.txt')); + + $cache->update($cache->getId('test.txt'), ['permissions' => \OCP\Constants::PERMISSION_READ]); + $this->assertEquals(\OCP\Constants::PERMISSION_READ, $instance->getPermissions('test.txt')); + + $this->assertTrue($instance->copy('test.txt', 'new.txt')); + + $this->assertTrue($cache->inCache('new.txt')); + $this->assertEquals(\OCP\Constants::PERMISSION_ALL, $instance->getPermissions('new.txt')); + } }