Merge pull request #41565 from nextcloud/fix/object-storage-inconsite…

…nt-behavior fix(ObjectStore): Make copying behavior consistent with local storage
nextcloud · Nov 22, 2023 · 9c3321c · 9c3321c
2 parents 8877f9b + 5172baa
commit 9c3321c
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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'));
+	}
 }