From 3e11707cd89617cf19a283d88e8ca9745bc636a5 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 12 Dec 2017 10:48:10 +0100 Subject: [PATCH] Check trashbin perms before moving to trash Check whether the target trashbin is writable (ex: guests). In the case where a guest deletes a file as recipient, the owner would get a copy in their trash. The logic here will detect that the guests' trashbin is not writeable so it will fall back to simply moving directly to the owner's trashbin. --- apps/files_trashbin/lib/Trashbin.php | 23 ++++- apps/files_trashbin/tests/StorageTest.php | 103 ++++++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-) diff --git a/apps/files_trashbin/lib/Trashbin.php b/apps/files_trashbin/lib/Trashbin.php index 05ff6d2b27c9..5f94aef9edb7 100644 --- a/apps/files_trashbin/lib/Trashbin.php +++ b/apps/files_trashbin/lib/Trashbin.php @@ -141,11 +141,23 @@ public static function getLocation($user, $filename, $timestamp) { } } + /** + * Sets up the trashbin for the given user + * + * @param string $user user id + * @return bool true if trashbin is setup and usable, false otherwise + */ private static function setUpTrash($user) { $view = new View('/' . $user); if (!$view->is_dir('files_trashbin')) { $view->mkdir('files_trashbin'); } + + if (!$view->isUpdatable('files_trashbin')) { + // no trashbin access or denied + return false; + } + if (!$view->is_dir('files_trashbin/files')) { $view->mkdir('files_trashbin/files'); } @@ -155,6 +167,8 @@ private static function setUpTrash($user) { if (!$view->is_dir('files_trashbin/keys')) { $view->mkdir('files_trashbin/keys'); } + + return true; } @@ -249,7 +263,14 @@ public static function move2trash($file_path) { return true; } - self::setUpTrash($user); + if (!self::setUpTrash($user)) { + // trashbin not usable for user (ex: guest), switch to owner only + $user = $owner; + if (!self::setUpTrash($owner)) { + // nothing to do as no trash is available anywheree + return true; + } + } if ($owner !== $user) { // also setup for owner self::setUpTrash($owner); diff --git a/apps/files_trashbin/tests/StorageTest.php b/apps/files_trashbin/tests/StorageTest.php index 92c20bb97b0c..5220674c2078 100644 --- a/apps/files_trashbin/tests/StorageTest.php +++ b/apps/files_trashbin/tests/StorageTest.php @@ -194,6 +194,109 @@ public function testCrossStorageDeleteFolder() { $this->assertEquals('subfile.txt', $name); } + /** + * Test that deleted folder appear in the trashbin of both owner and recipient + */ + public function testDeleteFolderAsRecipient() { + $this->userView->mkdir('share'); + $this->userView->mkdir('share/folder'); + $this->userView->file_put_contents('share/folder/test.txt', 'Yarrr! Content!'); + + $originalFileId = $this->userView->getFileInfo('share/folder/test.txt')->getId(); + + $recipientUser = $this->getUniqueId('recipient_'); + \OC::$server->getUserManager()->createUser($recipientUser, $recipientUser); + + $node = \OC::$server->getUserFolder($this->user)->get('share'); + $share = \OC::$server->getShareManager()->newShare(); + $share->setNode($node) + ->setShareType(\OCP\Share::SHARE_TYPE_USER) + ->setSharedBy($this->user) + ->setSharedWith($recipientUser) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + \OC::$server->getShareManager()->createShare($share); + + $this->loginAsUser($recipientUser); + + // delete as recipient + $recipientView = new View('/' . $recipientUser . '/files'); + $recipientView->rmdir('share/folder'); + + // check if folder is in trashbin for owner + $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/'); + $this->assertCount(1, $results); + $name = $results[0]->getName(); + $this->assertEquals('folder', substr($name, 0, strrpos($name, '.'))); + + // check if file is in trashbin for owner and has the same file id + $info = $this->rootView->getFileInfo($this->user . '/files_trashbin/files/' . $name . '/test.txt'); + $this->assertNotNull($info); + $this->assertEquals($originalFileId, $info->getId()); + + // check if folder is in trashbin for recipient + $results = $this->rootView->getDirectoryContent($recipientUser . '/files_trashbin/files/'); + $this->assertCount(1, $results); + $name = $results[0]->getName(); + $this->assertEquals('folder', substr($name, 0, strrpos($name, '.'))); + + // check if file has a copy in trashbin for recipient (different file id) + $info = $this->rootView->getFileInfo($recipientUser . '/files_trashbin/files/' . $name . '/test.txt'); + $this->assertNotNull($info); + $this->assertNotEquals($originalFileId, $info->getId()); + } + + /** + * Test that deleted folder appear only in the trashbin of owner when recipient + * has a read-only access home storage + */ + public function testDeleteFolderAsReadOnlyRecipient() { + $readOnlyGroups = \OC::$server->getConfig()->getAppValue('core', 'read_only_groups', null); + \OC::$server->getConfig()->setAppValue('core', 'read_only_groups', '["rogroup"]'); + + $this->userView->mkdir('share'); + $this->userView->mkdir('share/folder'); + $this->userView->file_put_contents('share/folder/test.txt', 'Yarrr! Content!'); + + $originalFileId = $this->userView->getFileInfo('share/folder/test.txt')->getId(); + + $recipientUser = $this->getUniqueId('recipient_'); + $recipientUserObject = \OC::$server->getUserManager()->createUser($recipientUser, $recipientUser); + $roGroupObject = \OC::$server->getGroupManager()->createGroup('rogroup'); + $roGroupObject->addUser($recipientUserObject); + + $node = \OC::$server->getUserFolder($this->user)->get('share'); + $share = \OC::$server->getShareManager()->newShare(); + $share->setNode($node) + ->setShareType(\OCP\Share::SHARE_TYPE_USER) + ->setSharedBy($this->user) + ->setSharedWith($recipientUser) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + \OC::$server->getShareManager()->createShare($share); + + $this->loginAsUser($recipientUser); + + // delete as recipient + $recipientView = new View('/' . $recipientUser . '/files'); + $recipientView->rmdir('share/folder'); + + // check if folder is in trashbin for owner + $results = $this->rootView->getDirectoryContent($this->user . '/files_trashbin/files/'); + $this->assertCount(1, $results); + $name = $results[0]->getName(); + $this->assertEquals('folder', substr($name, 0, strrpos($name, '.'))); + + // check if file is in trashbin for owner and has the same file id + $info = $this->rootView->getFileInfo($this->user . '/files_trashbin/files/' . $name . '/test.txt'); + $this->assertNotNull($info); + $this->assertEquals($originalFileId, $info->getId()); + + // check that folder is NOT in trashbin for recipient + $this->assertFalse($this->rootView->file_exists($recipientUser . '/files_trashbin')); + + \OC::$server->getConfig()->setAppValue('core', 'read_only_groups', $readOnlyGroups); + $roGroupObject->delete(); + } + /** * Test that deleted versions properly land in the trashbin. */