From 709691548dbf72c5763879136f3a0ba4e8a0c135 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 23 Jun 2014 16:14:52 +0200 Subject: [PATCH 1/3] always use oc filesystem for rename operation --- apps/files_encryption/hooks/hooks.php | 51 +++++++++------------------ apps/files_encryption/lib/helper.php | 24 +++++++++++++ 2 files changed, 41 insertions(+), 34 deletions(-) diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php index df101acab9..3625d5a09f 100644 --- a/apps/files_encryption/hooks/hooks.php +++ b/apps/files_encryption/hooks/hooks.php @@ -464,61 +464,44 @@ class Hooks { $newShareKeyPath = $ownerNew . '/files_encryption/share-keys/' . $pathNew; } - // add key ext if this is not an folder + // create new key folders if it doesn't exists + if (!$view->file_exists(dirname($newShareKeyPath))) { + $view->mkdir(dirname($newShareKeyPath)); + } + if (!$view->file_exists(dirname($newKeyfilePath))) { + $view->mkdir(dirname($newKeyfilePath)); + } + + // handle share keys if (!$view->is_dir($oldKeyfilePath)) { $oldKeyfilePath .= '.key'; $newKeyfilePath .= '.key'; // handle share-keys - $localKeyPath = $view->getLocalFile($oldShareKeyPath); - $escapedPath = Helper::escapeGlobPattern($localKeyPath); - $matches = glob($escapedPath . '*.shareKey'); + $matches = Helper::findShareKeys($oldShareKeyPath, $view); foreach ($matches as $src) { $dst = \OC\Files\Filesystem::normalizePath(str_replace($pathOld, $pathNew, $src)); - - // create destination folder if not exists - if (!file_exists(dirname($dst))) { - mkdir(dirname($dst), 0750, true); - } - - rename($src, $dst); + $view->rename($src, $dst); } } else { // handle share-keys folders - - // create destination folder if not exists - if (!$view->file_exists(dirname($newShareKeyPath))) { - mkdir($view->getLocalFile($newShareKeyPath), 0750, true); - } - $view->rename($oldShareKeyPath, $newShareKeyPath); } // Rename keyfile so it isn't orphaned if ($view->file_exists($oldKeyfilePath)) { - - // create destination folder if not exists - if (!$view->file_exists(dirname($newKeyfilePath))) { - mkdir(dirname($view->getLocalFile($newKeyfilePath)), 0750, true); - } - $view->rename($oldKeyfilePath, $newKeyfilePath); } - // build the path to the file - $newPath = '/' . $ownerNew . '/files' . $pathNew; + // update share keys + $sharingEnabled = \OCP\Share::isEnabled(); - if ($util->fixFileSize($newPath)) { - // get sharing app state - $sharingEnabled = \OCP\Share::isEnabled(); + // get users + $usersSharing = $util->getSharingUsersArray($sharingEnabled, $pathNew); - // get users - $usersSharing = $util->getSharingUsersArray($sharingEnabled, $pathNew); - - // update sharing-keys - $util->setSharedFileKeyfiles($session, $usersSharing, $pathNew); - } + // update sharing-keys + $util->setSharedFileKeyfiles($session, $usersSharing, $pathNew); \OC_FileProxy::$enabled = $proxyStatus; } diff --git a/apps/files_encryption/lib/helper.php b/apps/files_encryption/lib/helper.php index 2684bf7be3..c6f18602b2 100755 --- a/apps/files_encryption/lib/helper.php +++ b/apps/files_encryption/lib/helper.php @@ -438,6 +438,30 @@ class Helper { return preg_replace('/(\*|\?|\[)/', '[$1]', $path); } + /** + * find all share keys for a given file + * @param string $path to the file + * @param \OC\Files\View $view view, relative to data/ + */ + public static function findShareKeys($path, $view) { + $result = array(); + $pathinfo = pathinfo($path); + $dirContent = $view->opendir($pathinfo['dirname']); + + if (is_resource($dirContent)) { + while (($file = readdir($dirContent)) !== false) { + if (!\OC\Files\Filesystem::isIgnoredDir($file)) { + if (preg_match("/" . $pathinfo['filename'] . ".(.*).shareKey/", $file)) { + $result[] = $pathinfo['dirname'] . '/' . $file; + } + } + } + closedir($dirContent); + } + + return $result; + } + /** * remember from which file the tmp file (getLocalFile() call) was created * @param string $tmpFile path of tmp file From 3d0805f27d1d84399ed87403f33cfbde556ae69a Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 23 Jun 2014 17:13:56 +0200 Subject: [PATCH 2/3] always use a \OC\Files\View --- apps/files_encryption/lib/helper.php | 10 +--- apps/files_encryption/lib/keymanager.php | 59 +++++++++------------- apps/files_encryption/tests/hooks.php | 2 +- apps/files_encryption/tests/keymanager.php | 59 +++++++++++----------- 4 files changed, 56 insertions(+), 74 deletions(-) diff --git a/apps/files_encryption/lib/helper.php b/apps/files_encryption/lib/helper.php index c6f18602b2..fed0788028 100755 --- a/apps/files_encryption/lib/helper.php +++ b/apps/files_encryption/lib/helper.php @@ -429,19 +429,11 @@ class Helper { return $config; } - /** - * glob uses different pattern than regular expressions, escape glob pattern only - * @param string $path unescaped path - * @return string path - */ - public static function escapeGlobPattern($path) { - return preg_replace('/(\*|\?|\[)/', '[$1]', $path); - } - /** * find all share keys for a given file * @param string $path to the file * @param \OC\Files\View $view view, relative to data/ + * @return array list of files, path relative to data/ */ public static function findShareKeys($path, $view) { $result = array(); diff --git a/apps/files_encryption/lib/keymanager.php b/apps/files_encryption/lib/keymanager.php index 98986d1486..e71fec5685 100755 --- a/apps/files_encryption/lib/keymanager.php +++ b/apps/files_encryption/lib/keymanager.php @@ -133,20 +133,7 @@ class Keymanager { $basePath = '/' . $owner . '/files_encryption/keyfiles'; } - $targetPath = self::keySetPreparation($view, $filename, $basePath, $owner); - - if (!$view->is_dir($basePath . '/' . $targetPath)) { - - // create all parent folders - $info = pathinfo($basePath . '/' . $targetPath); - $keyfileFolderName = $view->getLocalFolder($info['dirname']); - - if (!file_exists($keyfileFolderName)) { - - mkdir($keyfileFolderName, 0750, true); - - } - } + $targetPath = self::keySetPreparation($view, $filename, $basePath); // try reusing key file if part file if (Helper::isPartialFilePath($targetPath)) { @@ -281,8 +268,9 @@ class Keymanager { $proxyStatus = \OC_FileProxy::$enabled; \OC_FileProxy::$enabled = false; - if (!$view->file_exists('')) + if (!$view->file_exists('')) { $view->mkdir(''); + } $result = $view->file_put_contents($user . '.private.key', $key); @@ -340,7 +328,7 @@ class Keymanager { $basePath = '/' . $owner . '/files_encryption/share-keys'; } - $shareKeyPath = self::keySetPreparation($view, $filename, $basePath, $owner); + $shareKeyPath = self::keySetPreparation($view, $filename, $basePath); $result = true; @@ -466,8 +454,7 @@ class Keymanager { if ($view->is_dir($shareKeyPath)) { - $localPath = \OC\Files\Filesystem::normalizePath($view->getLocalFolder($shareKeyPath)); - self::recursiveDelShareKeys($localPath, $userIds); + self::recursiveDelShareKeys($shareKeyPath, $userIds, $view); } else { @@ -491,23 +478,25 @@ class Keymanager { * @param string $dir directory * @param array $userIds user ids for which the share keys should be deleted */ - private static function recursiveDelShareKeys($dir, $userIds) { - foreach ($userIds as $userId) { - $extension = '.' . $userId . '.shareKey'; - $escapedDir = Helper::escapeGlobPattern($dir); - $escapedExtension = Helper::escapeGlobPattern($extension); - $matches = glob($escapedDir . '/*' . $escapedExtension); - } - /** @var $matches array */ - foreach ($matches as $ma) { - if (!unlink($ma)) { - \OCP\Util::writeLog('Encryption library', - 'Could not delete shareKey; does not exist: "' . $ma . '"', \OCP\Util::ERROR); + private static function recursiveDelShareKeys($dir, $userIds, $view) { + + $dirContent = $view->opendir($dir); + + if (is_resource($dirContent)) { + while (($file = readdir($dirContent)) !== false) { + if (!\OC\Files\Filesystem::isIgnoredDir($file)) { + if ($view->is_dir($dir . '/' . $file)) { + self::recursiveDelShareKeys($dir . '/' . $file, $userIds, $view); + } else { + foreach ($userIds as $userId) { + if (preg_match("/(.*)." . $userId . ".shareKey/", $file)) { + $view->unlink($dir . '/' . $file); + } + } + } + } } - } - $subdirs = glob($escapedDir . '/*', GLOB_ONLYDIR); - foreach ($subdirs as $subdir) { - self::recursiveDelShareKeys($subdir, $userIds); + closedir($dirContent); } } @@ -516,7 +505,7 @@ class Keymanager { * @param string|boolean $path * @param string $basePath */ - protected static function keySetPreparation(\OC\Files\View $view, $path, $basePath, $userId) { + protected static function keySetPreparation(\OC\Files\View $view, $path, $basePath) { $targetPath = ltrim($path, '/'); diff --git a/apps/files_encryption/tests/hooks.php b/apps/files_encryption/tests/hooks.php index 79699a3ef3..5eda8df01b 100644 --- a/apps/files_encryption/tests/hooks.php +++ b/apps/files_encryption/tests/hooks.php @@ -307,7 +307,7 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase { $this->assertTrue($this->rootView->is_dir('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder . '/' . $this->folder)); - // move the file out of the shared folder + // move the file to the sub-subfolder $root = $this->rootView->getRoot(); $this->rootView->chroot('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/'); $this->rootView->rename($this->filename, '/' . $this->folder . '/' . $this->folder . '/' . $this->filename); diff --git a/apps/files_encryption/tests/keymanager.php b/apps/files_encryption/tests/keymanager.php index 0c025443cd..7c36a4689b 100644 --- a/apps/files_encryption/tests/keymanager.php +++ b/apps/files_encryption/tests/keymanager.php @@ -206,43 +206,44 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase { */ function testRecursiveDelShareKeys() { - // generate filename - $filename = '/tmp-' . uniqid() . '.txt'; - // create folder structure - $this->view->mkdir('/'.Test_Encryption_Keymanager::TEST_USER.'/files/folder1'); - $this->view->mkdir('/'.Test_Encryption_Keymanager::TEST_USER.'/files/folder1/subfolder'); - $this->view->mkdir('/'.Test_Encryption_Keymanager::TEST_USER.'/files/folder1/subfolder/subsubfolder'); + $this->view->mkdir('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1'); + $this->view->mkdir('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder'); + $this->view->mkdir('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/subsubfolder'); - // enable encryption proxy - $proxyStatus = \OC_FileProxy::$enabled; - \OC_FileProxy::$enabled = true; + // create some dummy share keys + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.user1.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file2.user2.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file2.user3.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/file2.user3.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/subsubfolder/file1.user1.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/subsubfolder/file2.user2.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/subsubfolder/file2.user3.shareKey', 'data'); - // save file with content - $cryptedFile = file_put_contents('crypt:///'.Test_Encryption_Keymanager::TEST_USER.'/files/folder1/subfolder/subsubfolder' . $filename, $this->dataShort); + // recursive delete share keys from user1 and user2 + Encryption\Keymanager::delShareKey($this->view, array('user1', 'user2'), '/folder1/'); - // test that data was successfully written - $this->assertTrue(is_int($cryptedFile)); - - // change encryption proxy to previous state - \OC_FileProxy::$enabled = $proxyStatus; - - // recursive delete keys - Encryption\Keymanager::delShareKey($this->view, array('admin'), '/folder1/'); - - // check if share key not exists + // check if share keys from user1 and user2 are deleted $this->assertFalse($this->view->file_exists( - '/admin/files_encryption/share-keys/folder1/subfolder/subsubfolder/' . $filename . '.admin.shareKey')); + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.user1.shareKey')); + $this->assertFalse($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file2.user2.shareKey')); + $this->assertFalse($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/subsubfolder/file1.user1.shareKey')); + $this->assertFalse($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/subsubfolder/file2.user2.shareKey')); - // enable encryption proxy - $proxyStatus = \OC_FileProxy::$enabled; - \OC_FileProxy::$enabled = true; + // check if share keys from user3 still exists + $this->assertTrue($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file2.user3.shareKey')); + $this->assertTrue($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/subsubfolder/file2.user3.shareKey')); + $this->assertTrue($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/file2.user3.shareKey')); // cleanup - $this->view->deleteAll('/admin/files/folder1'); + $this->view->deleteAll('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys'); - // change encryption proxy to previous state - \OC_FileProxy::$enabled = $proxyStatus; } function testKeySetPreperation() { @@ -283,6 +284,6 @@ class TestProtectedKeymanagerMethods extends \OCA\Encryption\Keymanager { * @param string $basePath */ public static function testKeySetPreperation($view, $path, $basePath) { - return self::keySetPreparation($view, $path, $basePath, ''); + return self::keySetPreparation($view, $path, $basePath); } } From d3d1955ef492e24d75c72d0dc920f3c6c3951a4b Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 25 Jun 2014 12:38:26 +0200 Subject: [PATCH 3/3] fix unit tests --- apps/files_encryption/tests/keymanager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/files_encryption/tests/keymanager.php b/apps/files_encryption/tests/keymanager.php index 7c36a4689b..3339f8c73e 100644 --- a/apps/files_encryption/tests/keymanager.php +++ b/apps/files_encryption/tests/keymanager.php @@ -248,14 +248,14 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase { function testKeySetPreperation() { $basePath = '/'.Test_Encryption_Keymanager::TEST_USER.'/files'; - $path = '/folder1/subfolder/subsubfolder'; + $path = '/folder1/subfolder/subsubfolder/file.txt'; $this->assertFalse($this->view->is_dir($basePath . '/testKeySetPreperation')); $result = TestProtectedKeymanagerMethods::testKeySetPreperation($this->view, $path, $basePath); // return path without leading slash - $this->assertSame('folder1/subfolder/subsubfolder', $result); + $this->assertSame('folder1/subfolder/subsubfolder/file.txt', $result); // check if directory structure was created $this->assertTrue($this->view->is_dir($basePath . '/folder1/subfolder/subsubfolder'));