Skip to content

Commit 5172baa

Browse files
committed
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 <opensource@fthiessen.de>
1 parent 158aedb commit 5172baa

File tree

3 files changed

+47
-2
lines changed

3 files changed

+47
-2
lines changed

lib/private/Files/ObjectStore/ObjectStoreStorage.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common implements IChunkedFil
6868

6969
private $logger;
7070

71+
private bool $handleCopiesAsOwned;
72+
7173
/** @var bool */
7274
protected $validateWrites = true;
7375

@@ -88,6 +90,7 @@ public function __construct($params) {
8890
if (isset($params['validateWrites'])) {
8991
$this->validateWrites = (bool)$params['validateWrites'];
9092
}
93+
$this->handleCopiesAsOwned = (bool)($params['handleCopiesAsOwned'] ?? false);
9194

9295
$this->logger = \OC::$server->getLogger();
9396
}
@@ -651,6 +654,10 @@ private function copyFile(ICacheEntry $sourceEntry, string $to) {
651654

652655
try {
653656
$this->objectStore->copyObject($sourceUrn, $targetUrn);
657+
if ($this->handleCopiesAsOwned) {
658+
// 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 !
659+
$cache->update($targetId, ['permissions' => \OCP\Constants::PERMISSION_ALL]);
660+
}
654661
} catch (\Exception $e) {
655662
$cache->remove($to);
656663

lib/public/Files/Cache/ICacheEntry.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ public function getStorageMTime();
123123
public function getEtag();
124124

125125
/**
126-
* Get the permissions for the file stored as bitwise combination of \OCP\PERMISSION_READ, \OCP\PERMISSION_CREATE
127-
* \OCP\PERMISSION_UPDATE, \OCP\PERMISSION_DELETE and \OCP\PERMISSION_SHARE
126+
* Get the permissions for the file stored as bitwise combination of \OCP\Constants::PERMISSION_READ, \OCP\Constants::PERMISSION_CREATE
127+
* \OCP\Constants::PERMISSION_UPDATE, \OCP\Constants::PERMISSION_DELETE and \OCP\Constants::PERMISSION_SHARE
128128
*
129129
* @return int
130130
* @since 9.0.0

tests/lib/Files/ObjectStore/ObjectStoreStorageTest.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,4 +237,42 @@ public function testCopyBetweenJails() {
237237
$this->assertEquals('2', $this->instance->file_get_contents('b/target/sub/2.txt'));
238238
$this->assertEquals('3', $this->instance->file_get_contents('b/target/sub/3.txt'));
239239
}
240+
241+
public function testCopyPreservesPermissions() {
242+
$cache = $this->instance->getCache();
243+
244+
$this->instance->file_put_contents('test.txt', 'foo');
245+
$this->assertTrue($cache->inCache('test.txt'));
246+
247+
$cache->update($cache->getId('test.txt'), ['permissions' => \OCP\Constants::PERMISSION_READ]);
248+
$this->assertEquals(\OCP\Constants::PERMISSION_READ, $this->instance->getPermissions('test.txt'));
249+
250+
$this->assertTrue($this->instance->copy('test.txt', 'new.txt'));
251+
252+
$this->assertTrue($cache->inCache('new.txt'));
253+
$this->assertEquals(\OCP\Constants::PERMISSION_READ, $this->instance->getPermissions('new.txt'));
254+
}
255+
256+
/**
257+
* Test that copying files will drop permissions like local storage does
258+
* TODO: Drop this and fix local storage
259+
*/
260+
public function testCopyGrantsPermissions() {
261+
$config['objectstore'] = $this->objectStorage;
262+
$config['handleCopiesAsOwned'] = true;
263+
$instance = new ObjectStoreStorageOverwrite($config);
264+
265+
$cache = $instance->getCache();
266+
267+
$instance->file_put_contents('test.txt', 'foo');
268+
$this->assertTrue($cache->inCache('test.txt'));
269+
270+
$cache->update($cache->getId('test.txt'), ['permissions' => \OCP\Constants::PERMISSION_READ]);
271+
$this->assertEquals(\OCP\Constants::PERMISSION_READ, $instance->getPermissions('test.txt'));
272+
273+
$this->assertTrue($instance->copy('test.txt', 'new.txt'));
274+
275+
$this->assertTrue($cache->inCache('new.txt'));
276+
$this->assertEquals(\OCP\Constants::PERMISSION_ALL, $instance->getPermissions('new.txt'));
277+
}
240278
}

0 commit comments

Comments
 (0)