Merge pull request #63414 from nextcloud/backport/53733/stable30

[stable30] fix: give target file all permissions on copy

Author: susnux

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;
@@ -1162,6 +1163,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()) {

lib/private/Files/ObjectStore/ObjectStoreStorage.php

@@ -34,8 +34,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;
 
@@ -60,7 +58,6 @@ public function __construct($params) {
 		if (isset($params['validateWrites'])) {
 			$this->validateWrites = (bool) $params['validateWrites'];
 		}
-		$this->handleCopiesAsOwned = (bool) ($params['handleCopiesAsOwned'] ?? false);
 
 		$this->logger = \OCP\Server::get(LoggerInterface::class);
 	}
@@ -737,10 +734,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);
 

tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php

@@ -226,28 +226,8 @@ public function testCopyBetweenJails() {
 		$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();
@@ -261,7 +241,7 @@ public function testCopyGrantsPermissions() {
 		$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 {