[Pkg-owncloud-commits] [owncloud] 133/258: Fix share key finding algorithm in various cases

David Prévot taffit at moszumanska.debian.org
Sat Oct 11 17:22:29 UTC 2014


This is an automated email from the git hooks/post-receive script.

taffit pushed a commit to branch master
in repository owncloud.

commit 85d74923980fdbef0e2945fabf85a849890cb88a
Author: Vincent Petry <pvince81 at owncloud.com>
Date:   Wed Sep 17 18:50:29 2014 +0200

    Fix share key finding algorithm in various cases
    
    Instead of inaccurate pattern matching, use the list of users who we
    know have access to the file to build the list of share keys.
    
    This covers the following cases:
    
    - Move/copy files into a subfolder within a share
    - Unsharing from a user
    - Deleting files directlry / moving share keys to trashbin
    
    Backport of 1e631754d78e98d74ba0d3fb477d5eb815e9dfb3 from master
---
 apps/files_encryption/hooks/hooks.php      |   8 ++-
 apps/files_encryption/lib/helper.php       |  46 ++++++++-----
 apps/files_encryption/lib/keymanager.php   |  54 +++++++++------
 apps/files_encryption/tests/helper.php     |  56 +++++++++++++++
 apps/files_encryption/tests/hooks.php      | 105 +++++++++++++++++++----------
 apps/files_encryption/tests/keymanager.php |  60 +++++++++++++----
 apps/files_encryption/tests/share.php      |  57 ++++++++++++++++
 apps/files_encryption/tests/trashbin.php   |  89 +++++++++++++++++++-----
 apps/files_encryption/tests/util.php       |  16 +++--
 apps/files_trashbin/lib/trashbin.php       |   6 +-
 10 files changed, 384 insertions(+), 113 deletions(-)

diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php
index f38380b..89bf45b 100644
--- a/apps/files_encryption/hooks/hooks.php
+++ b/apps/files_encryption/hooks/hooks.php
@@ -537,7 +537,13 @@ class Hooks {
 			$newKeyfilePath .= '.key';
 
 			// handle share-keys
-			$matches = Helper::findShareKeys($oldShareKeyPath, $view);
+			$matches = Helper::findShareKeys($pathOld, $oldShareKeyPath, $view);
+			if (count($matches) === 0) {
+				\OC_Log::write(
+					'Encryption library', 'No share keys found for "' . $pathOld . '"',
+					\OC_Log::WARN
+				);
+			}
 			foreach ($matches as $src) {
 				$dst = \OC\Files\Filesystem::normalizePath(str_replace($pathOld, $pathNew, $src));
 				$view->$operation($src, $dst);
diff --git a/apps/files_encryption/lib/helper.php b/apps/files_encryption/lib/helper.php
index d427c51..ab19938 100755
--- a/apps/files_encryption/lib/helper.php
+++ b/apps/files_encryption/lib/helper.php
@@ -431,24 +431,40 @@ class Helper {
 
 	/**
 	 * 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/
+	 *
+	 * @param string $filePath path to the file name relative to the user's files dir
+	 * for example "subdir/filename.txt"
+	 * @param string $shareKeyPath share key prefix path relative to the user's data dir
+	 * for example "user1/files_encryption/share-keys/subdir/filename.txt"
+	 * @param \OC\Files\View $rootView root view, relative to data/
+	 * @return array list of share key files, path relative to data/$user
 	 */
-	public static function findShareKeys($path, $view) {
+	public static function findShareKeys($filePath, $shareKeyPath, $rootView) {
 		$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;
-					}
-				}
+
+		$user = \OCP\User::getUser();
+		$util = new Util($rootView, $user);
+		// get current sharing state
+		$sharingEnabled = \OCP\Share::isEnabled();
+
+		// get users sharing this file
+		$usersSharing = $util->getSharingUsersArray($sharingEnabled, $filePath);
+
+		$pathinfo = pathinfo($shareKeyPath);
+
+		$baseDir = $pathinfo['dirname'] . '/';
+		$fileName = $pathinfo['basename'];
+		foreach ($usersSharing as $user) {
+			$keyName = $fileName . '.' . $user . '.shareKey';
+			if ($rootView->file_exists($baseDir . $keyName)) {
+				$result[] = $baseDir . $keyName;
+			} else {
+				\OC_Log::write(
+					'Encryption library',
+					'No share key found for user "' . $user . '" for file "' . $pathOld . '"',
+					\OC_Log::WARN
+				);
 			}
-			closedir($dirContent);
 		}
 
 		return $result;
diff --git a/apps/files_encryption/lib/keymanager.php b/apps/files_encryption/lib/keymanager.php
index 931469f..9560126 100755
--- a/apps/files_encryption/lib/keymanager.php
+++ b/apps/files_encryption/lib/keymanager.php
@@ -459,13 +459,17 @@ class Keymanager {
 			\OCP\Util::writeLog('files_encryption', 'delAllShareKeys: delete share keys: ' . $baseDir . $filePath, \OCP\Util::DEBUG);
 			$result = $view->unlink($baseDir . $filePath);
 		} else {
-			$parentDir = dirname($baseDir . $filePath);
-			$filename = pathinfo($filePath, PATHINFO_BASENAME);
-			foreach($view->getDirectoryContent($parentDir) as $content) {
-				$path = $content['path'];
-				if (self::getFilenameFromShareKey($content['name'])  === $filename) {
-					\OCP\Util::writeLog('files_encryption', 'dellAllShareKeys: delete share keys: ' . '/' . $userId . '/' . $path, \OCP\Util::DEBUG);
-					$result &= $view->unlink('/' . $userId . '/' . $path);
+			$sharingEnabled = \OCP\Share::isEnabled();
+			$users = $util->getSharingUsersArray($sharingEnabled, $filePath);
+			foreach($users as $user) {
+				$keyName = $baseDir . $filePath . '.' . $user . '.shareKey';
+				if ($view->file_exists($keyName)) {
+					\OCP\Util::writeLog(
+						'files_encryption',
+						'dellAllShareKeys: delete share keys: "' . $keyName . '"',
+						\OCP\Util::DEBUG
+					);
+					$result &= $view->unlink($keyName);
 				}
 			}
 		}
@@ -539,17 +543,20 @@ class Keymanager {
 					if ($view->is_dir($dir . '/' . $file)) {
 						self::recursiveDelShareKeys($dir . '/' . $file, $userIds, $owner, $view);
 					} else {
-						$realFile = $realFileDir . self::getFilenameFromShareKey($file);
 						foreach ($userIds as $userId) {
-							if (preg_match("/(.*)." . $userId . ".shareKey/", $file)) {
-								if ($userId === $owner &&
-										$view->file_exists($realFile)) {
-									\OCP\Util::writeLog('files_encryption', 'original file still exists, keep owners share key!', \OCP\Util::ERROR);
-									continue;
-								}
-								\OCP\Util::writeLog('files_encryption', 'recursiveDelShareKey: delete share key: ' . $file, \OCP\Util::DEBUG);
-								$view->unlink($dir . '/' . $file);
+							$fileNameFromShareKey = self::getFilenameFromShareKey($file, $userId);
+							if (!$fileNameFromShareKey) {
+								continue;
 							}
+							$realFile = $realFileDir . $fileNameFromShareKey;
+
+							if ($userId === $owner &&
+									$view->file_exists($realFile)) {
+								\OCP\Util::writeLog('files_encryption', 'original file still exists, keep owners share key!', \OCP\Util::ERROR);
+								continue;
+							}
+							\OCP\Util::writeLog('files_encryption', 'recursiveDelShareKey: delete share key: ' . $file, \OCP\Util::DEBUG);
+							$view->unlink($dir . '/' . $file);
 						}
 					}
 				}
@@ -591,16 +598,19 @@ class Keymanager {
 	/**
 	 * extract filename from share key name
 	 * @param string $shareKey (filename.userid.sharekey)
+	 * @param string $userId
 	 * @return string|false filename or false
 	 */
-	protected static function getFilenameFromShareKey($shareKey) {
-		$parts = explode('.', $shareKey);
+	protected static function getFilenameFromShareKey($shareKey, $userId) {
+		$expectedSuffix = '.' . $userId . '.' . 'shareKey';
+		$suffixLen = strlen($expectedSuffix);
 
-		$filename = false;
-		if(count($parts) > 2) {
-			$filename = implode('.', array_slice($parts, 0, count($parts)-2));
+		$suffix = substr($shareKey, -$suffixLen);
+
+		if ($suffix !== $expectedSuffix) {
+			return false;
 		}
 
-		return $filename;
+		return substr($shareKey, 0, -$suffixLen);
 	}
 }
diff --git a/apps/files_encryption/tests/helper.php b/apps/files_encryption/tests/helper.php
index 582d814..b94fdeb 100644
--- a/apps/files_encryption/tests/helper.php
+++ b/apps/files_encryption/tests/helper.php
@@ -108,4 +108,60 @@ class Test_Encryption_Helper extends \PHPUnit_Framework_TestCase {
 		\Test_Encryption_Util::loginHelper(\Test_Encryption_Helper::TEST_ENCRYPTION_HELPER_USER1);
 	}
 
+	function userNamesProvider() {
+		return array(
+			array('testuser' . uniqid()),
+			array('user.name.with.dots'),
+		);
+	}
+
+	/**
+	 * Tests whether share keys can be found
+	 *
+	 * @dataProvider userNamesProvider
+	 */
+	function testFindShareKeys($userName) {
+		// note: not using dataProvider as we want to make
+		// sure that the correct keys are match and not any
+		// other ones that might happen to have similar names
+		\Test_Encryption_Util::setupHooks();
+		\Test_Encryption_Util::loginHelper($userName, true);
+		$testDir = 'testFindShareKeys' . uniqid() . '/';
+		$baseDir = $userName . '/files/' . $testDir;
+		$fileList = array(
+			't est.txt',
+			't est_.txt',
+			't est.doc.txt',
+			't est(.*).txt', // make sure the regexp is escaped
+			'multiple.dots.can.happen.too.txt',
+			't est.' . $userName . '.txt',
+			't est_.' . $userName . '.shareKey.txt',
+			'who would upload their.shareKey',
+			'user ones file.txt',
+			'user ones file.txt.backup',
+			'.t est.txt'
+		);
+
+		$rootView = new \OC\Files\View('/');
+		$rootView->mkdir($baseDir);
+		foreach ($fileList as $fileName) {
+			$rootView->file_put_contents($baseDir . $fileName, 'dummy');
+		}
+
+		$shareKeysDir = $userName . '/files_encryption/share-keys/' . $testDir;
+		foreach ($fileList as $fileName) {
+			// make sure that every file only gets its correct respective keys
+			$result = Encryption\Helper::findShareKeys($baseDir . $fileName, $shareKeysDir . $fileName, $rootView);
+			$this->assertEquals(
+				array($shareKeysDir . $fileName . '.' . $userName . '.shareKey'),
+				$result
+			);
+		}
+
+		// clean up
+		$rootView->unlink($baseDir);
+		\Test_Encryption_Util::logoutHelper();
+		\OC_User::deleteUser($userName);
+	}
+
 }
diff --git a/apps/files_encryption/tests/hooks.php b/apps/files_encryption/tests/hooks.php
index cc5b6d5..14d44fe 100644
--- a/apps/files_encryption/tests/hooks.php
+++ b/apps/files_encryption/tests/hooks.php
@@ -36,8 +36,8 @@ use OCA\Encryption;
  */
 class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase {
 
-	const TEST_ENCRYPTION_HOOKS_USER1 = "test-encryption-hooks-user1";
-	const TEST_ENCRYPTION_HOOKS_USER2 = "test-encryption-hooks-user2";
+	const TEST_ENCRYPTION_HOOKS_USER1 = "test-encryption-hooks-user1.dot";
+	const TEST_ENCRYPTION_HOOKS_USER2 = "test-encryption-hooks-user2.dot";
 
 	/**
 	 * @var \OC\Files\View
@@ -49,7 +49,26 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase {
 	public $filename;
 	public $folder;
 
+	private static $testFiles;
+
 	public static function setUpBeforeClass() {
+		// note: not using a data provider because these
+		// files all need to coexist to make sure the
+		// share keys are found properly (pattern matching)
+		self::$testFiles = array(
+			't est.txt',
+			't est_.txt',
+			't est.doc.txt',
+			't est(.*).txt', // make sure the regexp is escaped
+			'multiple.dots.can.happen.too.txt',
+			't est.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.txt',
+			't est_.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey.txt',
+			'who would upload their.shareKey',
+			'user ones file.txt',
+			'user ones file.txt.backup',
+			'.t est.txt'
+		);
+
 		// reset backend
 		\OC_User::clearBackends();
 		\OC_User::useBackend('database');
@@ -281,25 +300,33 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase {
 		}
 	}
 
-	/**
-	 * test rename operation
-	 */
 	function testRenameHook() {
+		// create all files to make sure all keys can coexist properly
+		foreach (self::$testFiles as $file) {
+			// save file with content
+			$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $file, $this->data);
 
-		// save file with content
-		$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->filename, $this->data);
+			// test that data was successfully written
+			$this->assertTrue(is_int($cryptedFile));
+		}
 
-		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		foreach (self::$testFiles as $file) {
+			$this->doTestRenameHook($file);
+		}
+	}
 
+	/**
+	 * test rename operation
+	 */
+	function doTestRenameHook($filename) {
 		// check if keys exists
 		$this->assertTrue($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/'
-			. $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
+			. $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
 
 		$this->assertTrue($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/'
-			. $this->filename . '.key'));
+			. $filename . '.key'));
 
 		// make subfolder and sub-subfolder
 		$this->rootView->mkdir('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder);
@@ -310,50 +337,58 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase {
 		// 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);
+		$this->rootView->rename($filename, '/' . $this->folder . '/' . $this->folder . '/' . $filename);
 		$this->rootView->chroot($root);
 
-		$this->assertFalse($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->filename));
-		$this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder . '/' . $this->folder . '/' . $this->filename));
+		$this->assertFalse($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $filename));
+		$this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder . '/' . $this->folder . '/' . $filename));
 
 		// keys should be renamed too
 		$this->assertFalse($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/'
-			. $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
+			. $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
 		$this->assertFalse($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/'
-			. $this->filename . '.key'));
+			. $filename . '.key'));
 
 		$this->assertTrue($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/' . $this->folder . '/' . $this->folder . '/'
-			. $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
+			. $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
 		$this->assertTrue($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/' . $this->folder . '/' . $this->folder . '/'
-			. $this->filename . '.key'));
+			. $filename . '.key'));
 
 		// cleanup
 		$this->rootView->unlink('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder);
 	}
 
-	/**
-	 * test rename operation
-	 */
 	function testCopyHook() {
+		// create all files to make sure all keys can coexist properly
+		foreach (self::$testFiles as $file) {
+			// save file with content
+			$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $file, $this->data);
 
-		// save file with content
-		$cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->filename, $this->data);
+			// test that data was successfully written
+			$this->assertTrue(is_int($cryptedFile));
+		}
 
-		// test that data was successfully written
-		$this->assertTrue(is_int($cryptedFile));
+		foreach (self::$testFiles as $file) {
+			$this->doTestCopyHook($file);
+		}
+	}
 
+	/**
+	 * test rename operation
+	 */
+	function doTestCopyHook($filename) {
 		// check if keys exists
 		$this->assertTrue($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/'
-			. $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
+			. $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
 
 		$this->assertTrue($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/'
-			. $this->filename . '.key'));
+			. $filename . '.key'));
 
 		// make subfolder and sub-subfolder
 		$this->rootView->mkdir('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder);
@@ -362,29 +397,29 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase {
 		$this->assertTrue($this->rootView->is_dir('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder . '/' . $this->folder));
 
 		// copy the file to the sub-subfolder
-		\OC\Files\Filesystem::copy($this->filename, '/' . $this->folder . '/' . $this->folder . '/' . $this->filename);
+		\OC\Files\Filesystem::copy($filename, '/' . $this->folder . '/' . $this->folder . '/' . $filename);
 
-		$this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->filename));
-		$this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder . '/' . $this->folder . '/' . $this->filename));
+		$this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $filename));
+		$this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder . '/' . $this->folder . '/' . $filename));
 
 		// keys should be copied too
 		$this->assertTrue($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/'
-			. $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
+			. $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
 		$this->assertTrue($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/'
-			. $this->filename . '.key'));
+			. $filename . '.key'));
 
 		$this->assertTrue($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/' . $this->folder . '/' . $this->folder . '/'
-			. $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
+			. $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey'));
 		$this->assertTrue($this->rootView->file_exists(
 			'/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/' . $this->folder . '/' . $this->folder . '/'
-			. $this->filename . '.key'));
+			. $filename . '.key'));
 
 		// cleanup
 		$this->rootView->unlink('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder);
-		$this->rootView->unlink('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->filename);
+		$this->rootView->unlink('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $filename);
 	}
 
 	/**
diff --git a/apps/files_encryption/tests/keymanager.php b/apps/files_encryption/tests/keymanager.php
index f908322..e8a9d7d 100644
--- a/apps/files_encryption/tests/keymanager.php
+++ b/apps/files_encryption/tests/keymanager.php
@@ -23,7 +23,7 @@ use OCA\Encryption;
  */
 class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase {
 
-	const TEST_USER = "test-keymanager-user";
+	const TEST_USER = "test-keymanager-user.dot";
 
 	public $userId;
 	public $pass;
@@ -135,15 +135,25 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase {
 		$this->assertArrayHasKey('key', $sslInfo);
 	}
 
+	function fileNameFromShareKeyProvider() {
+		return array(
+			array('file.user.shareKey', 'user', 'file'),
+			array('file.name.with.dots.user.shareKey', 'user', 'file.name.with.dots'),
+			array('file.name.user.with.dots.shareKey', 'user.with.dots', 'file.name'),
+			array('file.txt', 'user', false),
+			array('user.shareKey', 'user', false),
+		);
+	}
+
 	/**
 	 * @small
+	 *
+	 * @dataProvider fileNameFromShareKeyProvider
 	 */
-	function testGetFilenameFromShareKey() {
-		$this->assertEquals("file",
-				\TestProtectedKeymanagerMethods::testGetFilenameFromShareKey("file.user.shareKey"));
-		$this->assertEquals("file.name.with.dots",
-				\TestProtectedKeymanagerMethods::testGetFilenameFromShareKey("file.name.with.dots.user.shareKey"));
-		$this->assertFalse(\TestProtectedKeymanagerMethods::testGetFilenameFromShareKey("file.txt"));
+	function testGetFilenameFromShareKey($fileName, $user, $expectedFileName) {
+		$this->assertEquals($expectedFileName,
+			\TestProtectedKeymanagerMethods::testGetFilenameFromShareKey($fileName, $user)
+		);
 	}
 
 	/**
@@ -249,6 +259,12 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase {
 		$this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/existingFile.txt.user1.shareKey', 'data');
 		$this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/existingFile.txt.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey', 'data');
 		$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/file1.user1.test.shareKey', 'data');
+		$this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.test-keymanager-userxdot.shareKey', 'data');
+		$this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.userx.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey', 'data');
+		$this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.' . Test_Encryption_Keymanager::TEST_USER . '.userx.shareKey', 'data');
+		$this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.user1.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey', 'data');
+		$this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.' . Test_Encryption_Keymanager::TEST_USER . '.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');
@@ -279,6 +295,23 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase {
 		$this->assertTrue($this->view->file_exists(
 			'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/file2.user3.shareKey'));
 
+		// check if share keys for user or file with similar name 
+		$this->assertTrue($this->view->file_exists(
+			'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.user1.test.shareKey'));
+		$this->assertTrue($this->view->file_exists(
+			'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.test-keymanager-userxdot.shareKey'));
+		$this->assertTrue($this->view->file_exists(
+			'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.' . Test_Encryption_Keymanager::TEST_USER . '.userx.shareKey'));
+		// FIXME: this case currently cannot be distinguished, needs further fixing
+		/*
+		$this->assertTrue($this->view->file_exists(
+			'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.userx.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey'));
+		$this->assertTrue($this->view->file_exists(
+			'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.user1.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey'));
+		$this->assertTrue($this->view->file_exists(
+			'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.' . Test_Encryption_Keymanager::TEST_USER . '.user1.shareKey'));
+		 */
+
 		// owner key from existing file should still exists because the file is still there
 		$this->assertTrue($this->view->file_exists(
 			'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/existingFile.txt.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey'));
@@ -432,18 +465,19 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase {
 		$this->assertTrue($this->view->file_exists(
 				'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/existingFile.txt.user3.shareKey'));
 
-		// try to del all share keys froma file, should fail because the file still exists
+		// try to del all share keys from file, should succeed because the does not exist any more
 		$result2 = Encryption\Keymanager::delAllShareKeys($this->view, Test_Encryption_Keymanager::TEST_USER, 'folder1/nonexistingFile.txt');
 		$this->assertTrue($result2);
 
 		// check if share keys are really gone
 		$this->assertFalse($this->view->file_exists(
 				'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/nonexistingFile.txt.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey'));
-		$this->assertFalse($this->view->file_exists(
+		// check that it only deleted keys or users who had access, others remain
+		$this->assertTrue($this->view->file_exists(
 				'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/nonexistingFile.txt.user1.shareKey'));
-		$this->assertFalse($this->view->file_exists(
+		$this->assertTrue($this->view->file_exists(
 				'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/nonexistingFile.txt.user2.shareKey'));
-		$this->assertFalse($this->view->file_exists(
+		$this->assertTrue($this->view->file_exists(
 				'/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/nonexistingFile.txt.user3.shareKey'));
 
 		// cleanup
@@ -479,8 +513,8 @@ class TestProtectedKeymanagerMethods extends \OCA\Encryption\Keymanager {
 	/**
 	 * @param string $sharekey
 	 */
-	public static function testGetFilenameFromShareKey($sharekey) {
-		return self::getFilenameFromShareKey($sharekey);
+	public static function testGetFilenameFromShareKey($sharekey, $user) {
+		return self::getFilenameFromShareKey($sharekey, $user);
 	}
 
 	/**
diff --git a/apps/files_encryption/tests/share.php b/apps/files_encryption/tests/share.php
index cf5a122..dcf0eda 100755
--- a/apps/files_encryption/tests/share.php
+++ b/apps/files_encryption/tests/share.php
@@ -1059,9 +1059,66 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase {
 		// check if additional share key for user2 exists
 		$this->assertTrue($view->file_exists('files_encryption/share-keys' . $newFolder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey'));
 
+		// check that old keys were removed/moved properly
+		$this->assertFalse($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey'));
+
 		// tear down
 		\OC\Files\Filesystem::unlink($newFolder);
 		\OC\Files\Filesystem::unlink('/newfolder');
 	}
 
+	function testMoveFileToFolder() {
+
+		$view = new \OC\Files\View('/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1);
+
+		$filename = '/tmp-' . uniqid();
+		$folder = '/folder' . uniqid();
+
+		\OC\Files\Filesystem::mkdir($folder);
+
+		// Save long data as encrypted file using stream wrapper
+		$cryptedFile = \OC\Files\Filesystem::file_put_contents($folder . $filename, $this->dataShort);
+
+		// Test that data was successfully written
+		$this->assertTrue(is_int($cryptedFile));
+
+		// Get file decrypted contents
+		$decrypt = \OC\Files\Filesystem::file_get_contents($folder . $filename);
+
+		$this->assertEquals($this->dataShort, $decrypt);
+
+		$subFolder = $folder . '/subfolder' . uniqid();
+		\OC\Files\Filesystem::mkdir($subFolder);
+
+		// get the file info from previous created file
+		$fileInfo = \OC\Files\Filesystem::getFileInfo($folder);
+		$this->assertTrue($fileInfo instanceof \OC\Files\FileInfo);
+
+		// share the folder
+		\OCP\Share::shareItem('folder', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2, OCP\PERMISSION_ALL);
+
+		// check that the share keys exist
+		$this->assertTrue($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '.shareKey'));
+		$this->assertTrue($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey'));
+
+		// move the file into the subfolder
+		\OC\Files\Filesystem::rename($folder . $filename, $subFolder . $filename);
+
+		// Get file decrypted contents
+		$newDecrypt = \OC\Files\Filesystem::file_get_contents($subFolder . $filename);
+		$this->assertEquals($this->dataShort, $newDecrypt);
+
+		// check if additional share key for user2 exists
+		$this->assertTrue($view->file_exists('files_encryption/share-keys' . $subFolder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '.shareKey'));
+		$this->assertTrue($view->file_exists('files_encryption/share-keys' . $subFolder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey'));
+
+		// check that old keys were removed/moved properly
+		$this->assertFalse($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '.shareKey'));
+		$this->assertFalse($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey'));
+
+		// tear down
+		\OC\Files\Filesystem::unlink($subFolder);
+		\OC\Files\Filesystem::unlink($folder);
+	}
+
 }
diff --git a/apps/files_encryption/tests/trashbin.php b/apps/files_encryption/tests/trashbin.php
index a5479de..ae09744 100755
--- a/apps/files_encryption/tests/trashbin.php
+++ b/apps/files_encryption/tests/trashbin.php
@@ -120,24 +120,33 @@ class Test_Encryption_Trashbin extends \PHPUnit_Framework_TestCase {
 
 		// generate filename
 		$filename = 'tmp-' . uniqid() . '.txt';
+		$filename2 = $filename . '.backup'; // a second file with similar name
 
 		// save file with content
 		$cryptedFile = file_put_contents('crypt:///' .\Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1. '/files/'. $filename, $this->dataShort);
+		$cryptedFile2 = file_put_contents('crypt:///' .\Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1. '/files/'. $filename2, $this->dataShort);
 
 		// test that data was successfully written
 		$this->assertTrue(is_int($cryptedFile));
+		$this->assertTrue(is_int($cryptedFile2));
 
 		// check if key for admin exists
 		$this->assertTrue($this->view->file_exists(
 			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/keyfiles/' . $filename
 			. '.key'));
+		$this->assertTrue($this->view->file_exists(
+			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/keyfiles/' . $filename2
+			. '.key'));
 
 		// check if share key for admin exists
 		$this->assertTrue($this->view->file_exists(
 			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/'
 			. $filename . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey'));
+		$this->assertTrue($this->view->file_exists(
+			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/'
+			. $filename2 . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey'));
 
-		// delete file
+		// delete first file
 		\OC\FIles\Filesystem::unlink($filename);
 
 		// check if file not exists
@@ -154,6 +163,20 @@ class Test_Encryption_Trashbin extends \PHPUnit_Framework_TestCase {
 			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/'
 			. $filename . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey'));
 
+		// check that second file still exists
+		$this->assertTrue($this->view->file_exists(
+			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files/' . $filename2));
+
+		// check that key for second file still exists
+		$this->assertTrue($this->view->file_exists(
+			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/keyfiles/' . $filename2
+			. '.key'));
+
+		// check that share key for second file still exists
+		$this->assertTrue($this->view->file_exists(
+			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/'
+			. $filename2 . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey'));
+
 		// get files
 		$trashFiles = $this->view->getDirectoryContent(
 			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_trashbin/files/');
@@ -179,41 +202,75 @@ class Test_Encryption_Trashbin extends \PHPUnit_Framework_TestCase {
 		$this->assertTrue($this->view->file_exists(
 			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_trashbin/share-keys/' . $filename
 			. '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey.' . $trashFileSuffix));
-
-		// return filename for next test
-		return $filename . '.' . $trashFileSuffix;
 	}
 
 	/**
 	 * @medium
 	 * test restore file
-	 *
-	 * @depends testDeleteFile
 	 */
-	function testRestoreFile($filename) {
+	function testRestoreFile() {
+		// generate filename
+		$filename = 'tmp-' . uniqid() . '.txt';
+		$filename2 = $filename . '.backup'; // a second file with similar name
+
+		// save file with content
+		$cryptedFile = file_put_contents('crypt:///' .\Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1. '/files/'. $filename, $this->dataShort);
+		$cryptedFile2 = file_put_contents('crypt:///' .\Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1. '/files/'. $filename2, $this->dataShort);
+
+		// delete both files
+		\OC\Files\Filesystem::unlink($filename);
+		\OC\Files\Filesystem::unlink($filename2);
+
+		$trashFiles = $this->view->getDirectoryContent(
+			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_trashbin/files/');
+
+		$trashFileSuffix = null;
+		$trashFileSuffix2 = null;
+		// find created file with timestamp
+		foreach ($trashFiles as $file) {
+			if (strncmp($file['path'], $filename, strlen($filename))) {
+				$path_parts = pathinfo($file['name']);
+				$trashFileSuffix = $path_parts['extension'];
+			}
+			if (strncmp($file['path'], $filename2, strlen($filename2))) {
+				$path_parts = pathinfo($file['name']);
+				$trashFileSuffix2 = $path_parts['extension'];
+			}
+		}
 
 		// prepare file information
-		$path_parts = pathinfo($filename);
-		$trashFileSuffix = $path_parts['extension'];
 		$timestamp = str_replace('d', '', $trashFileSuffix);
-		$fileNameWithoutSuffix = str_replace('.' . $trashFileSuffix, '', $filename);
 
-		// restore file
-		$this->assertTrue(\OCA\Files_Trashbin\Trashbin::restore($filename, $fileNameWithoutSuffix, $timestamp));
+		// restore first file
+		$this->assertTrue(\OCA\Files_Trashbin\Trashbin::restore($filename . '.' . $trashFileSuffix, $filename, $timestamp));
 
 		// check if file exists
 		$this->assertTrue($this->view->file_exists(
-			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files/' . $fileNameWithoutSuffix));
+			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files/' . $filename));
 
 		// check if key for admin exists
 		$this->assertTrue($this->view->file_exists(
 			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/keyfiles/'
-			. $fileNameWithoutSuffix . '.key'));
+			. $filename . '.key'));
 
 		// check if share key for admin exists
 		$this->assertTrue($this->view->file_exists(
 			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/'
-			. $fileNameWithoutSuffix . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey'));
+			. $filename . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey'));
+
+		// check that second file was NOT restored
+		$this->assertFalse($this->view->file_exists(
+			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files/' . $filename2));
+
+		// check if key for admin exists
+		$this->assertFalse($this->view->file_exists(
+			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/keyfiles/'
+			. $filename2 . '.key'));
+
+		// check if share key for admin exists
+		$this->assertFalse($this->view->file_exists(
+			'/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/'
+			. $filename2 . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey'));
 	}
 
 	/**
@@ -242,7 +299,7 @@ class Test_Encryption_Trashbin extends \PHPUnit_Framework_TestCase {
 			. $filename . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey'));
 
 		// delete file
-		\OC\FIles\Filesystem::unlink($filename);
+		\OC\Files\Filesystem::unlink($filename);
 
 		// check if file not exists
 		$this->assertFalse($this->view->file_exists(
diff --git a/apps/files_encryption/tests/util.php b/apps/files_encryption/tests/util.php
index 8213890..bdd4a2f 100755
--- a/apps/files_encryption/tests/util.php
+++ b/apps/files_encryption/tests/util.php
@@ -53,12 +53,7 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase {
 		\OC_User::clearBackends();
 		\OC_User::useBackend('database');
 
-		// Filesystem related hooks
-		\OCA\Encryption\Helper::registerFilesystemHooks();
-
-		// clear and register hooks
-		\OC_FileProxy::clearProxies();
-		\OC_FileProxy::register(new OCA\Encryption\Proxy());
+		self::setupHooks();
 
 		// create test user
 		\Test_Encryption_Util::loginHelper(\Test_Encryption_Util::TEST_ENCRYPTION_UTIL_USER1, true);
@@ -134,6 +129,15 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase {
 		\OC_Group::deleteGroup(self::TEST_ENCRYPTION_UTIL_GROUP2);
 	}
 
+	public static function setupHooks() {
+		// Filesystem related hooks
+		\OCA\Encryption\Helper::registerFilesystemHooks();
+
+		// clear and register hooks
+		\OC_FileProxy::clearProxies();
+		\OC_FileProxy::register(new OCA\Encryption\Proxy());
+	}
+
 	/**
 	 * @medium
 	 * test that paths set during User construction are correct
diff --git a/apps/files_trashbin/lib/trashbin.php b/apps/files_trashbin/lib/trashbin.php
index 1edf51e..492a0e3 100644
--- a/apps/files_trashbin/lib/trashbin.php
+++ b/apps/files_trashbin/lib/trashbin.php
@@ -278,12 +278,8 @@ class Trashbin {
 				}
 				$rootView->rename($sharekeys, $user . '/files_trashbin/share-keys/' . $filename . '.d' . $timestamp);
 			} else {
-				// get local path to share-keys
-				$localShareKeysPath = $rootView->getLocalFile($sharekeys);
-				$escapedLocalShareKeysPath = preg_replace('/(\*|\?|\[)/', '[$1]', $localShareKeysPath);
-
 				// handle share-keys
-				$matches = glob($escapedLocalShareKeysPath . '*.shareKey');
+				$matches = \OCA\Encryption\Helper::findShareKeys($ownerPath, $sharekeys, $rootView);
 				foreach ($matches as $src) {
 					// get source file parts
 					$pathinfo = pathinfo($src);

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git



More information about the Pkg-owncloud-commits mailing list