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

David Prévot taffit at moszumanska.debian.org
Thu Nov 13 19:37:13 UTC 2014


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

taffit pushed a commit to annotated tag v6.0.6
in repository owncloud.

commit 828a939f96017184eb385f80243aae33e7ae8a5a
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
    
    Conflicts:
    	apps/files_encryption/lib/keymanager.php
    	apps/files_encryption/tests/hooks.php
    	apps/files_encryption/tests/keymanager.php
    	apps/files_encryption/tests/trashbin.php
---
 apps/files_encryption/hooks/hooks.php      |   8 ++-
 apps/files_encryption/lib/helper.php       |  46 ++++++++-----
 apps/files_encryption/lib/keymanager.php   |  55 +++++++++-------
 apps/files_encryption/tests/helper.php     |  56 ++++++++++++++++
 apps/files_encryption/tests/hooks.php      | 102 ++++++++++++++++++++---------
 apps/files_encryption/tests/keymanager.php |  64 ++++++++++++++----
 apps/files_encryption/tests/share.php      |  57 ++++++++++++++++
 apps/files_encryption/tests/trashbin.php   |  91 ++++++++++++++++++++-----
 apps/files_encryption/tests/util.php       |  16 +++--
 apps/files_trashbin/lib/trashbin.php       |   6 +-
 10 files changed, 389 insertions(+), 112 deletions(-)

diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php
index 44dcaae..50b21c6 100644
--- a/apps/files_encryption/hooks/hooks.php
+++ b/apps/files_encryption/hooks/hooks.php
@@ -636,7 +636,13 @@ class Hooks {
 
 
 			// 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 89399ab..787a6ea 100755
--- a/apps/files_encryption/lib/helper.php
+++ b/apps/files_encryption/lib/helper.php
@@ -424,24 +424,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 b4197a2..3d930a3 100755
--- a/apps/files_encryption/lib/keymanager.php
+++ b/apps/files_encryption/lib/keymanager.php
@@ -441,13 +441,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);
 				}
 			}
 		}
@@ -520,17 +524,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);
 						}
 					}
 				}
@@ -570,16 +577,18 @@ class Keymanager {
 	/**
 	 * @brief extract filename from share key name
 	 * @param string $shareKey (filename.userid.sharekey)
-	 * @return mixed filename or false
+	 * @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 4b46e97..f2d773e 100644
--- a/apps/files_encryption/tests/helper.php
+++ b/apps/files_encryption/tests/helper.php
@@ -101,4 +101,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 edd2854..1a03f34 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_FilesystemView
@@ -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');
@@ -274,21 +293,32 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase {
 	 * @brief 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);
@@ -299,50 +329,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);
@@ -351,29 +389,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 022dd78..10cce8e 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)
+		);
 	}
 
 	/**
@@ -217,6 +227,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');
@@ -247,6 +263,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'));
@@ -400,18 +433,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
@@ -424,7 +458,11 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase {
  * dummy class to access protected methods of \OCA\Encryption\Keymanager for testing
  */
 class TestProtectedKeymanagerMethods extends \OCA\Encryption\Keymanager {
-	public static function testGetFilenameFromShareKey($sharekey) {
-		return self::getFilenameFromShareKey($sharekey);
+
+	/**
+	 * @param string $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 cf52022..19886bd 100755
--- a/apps/files_encryption/tests/share.php
+++ b/apps/files_encryption/tests/share.php
@@ -1066,9 +1066,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 2f9ecfd..91c07d9 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
-	 * @brief test restore file
-	 *
-	 * @depends testDeleteFile
+	 * test restore file
 	 */
-	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 2f37171..a948732 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
 	 * @brief 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 6f24ae4..2537c9e 100644
--- a/apps/files_trashbin/lib/trashbin.php
+++ b/apps/files_trashbin/lib/trashbin.php
@@ -290,12 +290,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