From dcc554ce759cb7aa4eb1816c5dc5d30ef4590bb7 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 30 Jun 2025 18:15:08 +0200 Subject: [PATCH 1/3] fix: give target file all permissions on copy Signed-off-by: Robin Appelman Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- lib/private/Files/Cache/Cache.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index c41844775a1e2..c3181a6a377bf 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -14,6 +14,7 @@ use OC\Files\Search\SearchQuery; use OC\Files\Storage\Wrapper\Encryption; use OC\SystemConfig; +use OCP\Constants; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Cache\CacheEntryInsertedEvent; @@ -1169,6 +1170,12 @@ public function copyFromCache(ICache $sourceCache, ICacheEntry $sourceEntry, str throw new \RuntimeException('Invalid source cache entry on copyFromCache'); } $data = $this->cacheEntryToArray($sourceEntry); + // since we are essentially creating a new file, we don't have to obey the source permissions + if ($sourceEntry->getMimeType() === ICacheEntry::DIRECTORY_MIMETYPE) { + $data['permissions'] = Constants::PERMISSION_ALL; + } else { + $data['permissions'] = Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE; + } // when moving from an encrypted storage to a non-encrypted storage remove the `encrypted` mark if ($sourceCache instanceof Cache && $sourceCache->hasEncryptionWrapper() && !$this->hasEncryptionWrapper()) { From 610407abbab8bec760613a99b77915c785ad4bbc Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 10 Mar 2026 16:07:38 +0100 Subject: [PATCH 2/3] fix: make objectstore copy consistent with changed local storage behavior Signed-off-by: Robin Appelman Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- .../Files/ObjectStore/ObjectStoreStorage.php | 7 ------ .../ObjectStore/ObjectStoreStorageTest.php | 23 +------------------ 2 files changed, 1 insertion(+), 29 deletions(-) diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php index c7d4fe6eee5c4..b36cec172e167 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php +++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php @@ -35,8 +35,6 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFil private string $objectPrefix = 'urn:oid:'; private LoggerInterface $logger; - - private bool $handleCopiesAsOwned; protected bool $validateWrites = true; private bool $preserveCacheItemsOnDelete = false; @@ -61,7 +59,6 @@ public function __construct(array $parameters) { if (isset($parameters['validateWrites'])) { $this->validateWrites = (bool)$parameters['validateWrites']; } - $this->handleCopiesAsOwned = (bool)($parameters['handleCopiesAsOwned'] ?? false); $this->logger = \OCP\Server::get(LoggerInterface::class); } @@ -729,10 +726,6 @@ 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/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php b/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php index 508f328fae213..c3794dea1bebc 100644 --- a/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php +++ b/tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php @@ -224,29 +224,8 @@ public function testCopyBetweenJails(): void { $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(): void { - $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(): void { $config['objectstore'] = $this->objectStorage; - $config['handleCopiesAsOwned'] = true; $instance = new ObjectStoreStorageOverwrite($config); $cache = $instance->getCache(); @@ -260,7 +239,7 @@ public function testCopyGrantsPermissions(): void { $this->assertTrue($instance->copy('test.txt', 'new.txt')); $this->assertTrue($cache->inCache('new.txt')); - $this->assertEquals(\OCP\Constants::PERMISSION_ALL, $instance->getPermissions('new.txt')); + $this->assertEquals(\OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_CREATE, $instance->getPermissions('new.txt')); } public function testCopyFolderSize(): void { From 8359499e1be7fbf70a92a1bc09bd6cde04c988e8 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Wed, 19 Aug 2026 22:53:42 +0200 Subject: [PATCH 3/3] chore: resolve merge conflicts with tests Signed-off-by: Ferdinand Thiessen --- build/integration/dav_features/dav-v2-public.feature | 10 ++++++---- lib/private/Share20/Manager.php | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/build/integration/dav_features/dav-v2-public.feature b/build/integration/dav_features/dav-v2-public.feature index 9b36ef4ff20f9..6097f69627945 100644 --- a/build/integration/dav_features/dav-v2-public.feature +++ b/build/integration/dav_features/dav-v2-public.feature @@ -70,7 +70,8 @@ Feature: dav-v2-public And creating a new public chunking upload with id "chunking-public-copy" And uploading new public chunk file "1" with "AAAAA" to id "chunking-public-copy" When copying new public chunk file with id "chunking-public-copy" to "/target.txt" - Then the HTTP status code should be "405" + # Then the HTTP status code should be "405" + Then the HTTP status code should be "409" And Downloading file "/public-upload/target.txt" as "user0" Then Downloaded content should be "original content" @@ -87,9 +88,10 @@ Feature: dav-v2-public And creating a new public chunking upload with id "chunking-public-move" And uploading new public chunk file "1" with "AAAAA" to id "chunking-public-move" When moving new public chunk file with id "chunking-public-move" to "/target.txt" - Then the HTTP status code should be "204" - And Downloading file "/public-upload/target.txt" as "user0" - Then Downloaded content should be "AAAAA" + # Then the HTTP status code should be "204" + Then the HTTP status code should be "409" + # And Downloading file "/public-upload/target.txt" as "user0" + # Then Downloaded content should be "AAAAA" Scenario: Download a folder Given using new dav path diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index b22b7654ad51c..8c8cbb4cc60fd 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -317,7 +317,7 @@ protected function validateExpirationDateInternal(IShare $share) { if ($fullId === null && $expirationDate === null && $defaultExpireDate) { $expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone()); $expirationDate->setTime(23, 59, 59); - $days = (int) $this->config->getAppValue('core', $configProp, (string) $defaultExpireDays); + $days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays); if ($days > $defaultExpireDays) { $days = $defaultExpireDays; }