ceanup encryption code, improved return codes

apps/files_encryption/hooks/hooks.php

@@ -90,6 +90,8 @@ class Hooks {
 			return false;
 		}
 
+		$result = true;
+
 		// If migration not yet done
 		if ($ready) {
 
@@ -97,15 +99,12 @@ class Hooks {
 
 			// Set legacy encryption key if it exists, to support
 			// depreciated encryption system
-			if (
-				$userView->file_exists('encryption.key')
-				&& $encLegacyKey = $userView->file_get_contents('encryption.key')
-			) {
+			$encLegacyKey = $userView->file_get_contents('encryption.key');
+			if ($encLegacyKey) {
 
 				$plainLegacyKey = Crypt::legacyDecrypt($encLegacyKey, $params['password']);
 
 				$session->setLegacyKey($plainLegacyKey);
-
 			}
 
 			// Encrypt existing user files
@@ -113,26 +112,24 @@ class Hooks {
 				$result = $util->encryptAll('/' . $params['uid'] . '/' . 'files', $session->getLegacyKey(), $params['password']);
 			} catch (\Exception $ex) {
 				\OCP\Util::writeLog('Encryption library', 'Initial encryption failed! Error: ' . $ex->getMessage(), \OCP\Util::FATAL);
-				$util->resetMigrationStatus();
-				\OCP\User::logout();
 				$result = false;
 			}
 
 			if ($result) {
-
 				\OC_Log::write(
 						'Encryption library', 'Encryption of existing files belonging to "' . $params['uid'] . '" completed'
 						, \OC_Log::INFO
 					);
-
 				// Register successful migration in DB
 				$util->finishMigration();
-
+			} else  {
+				\OCP\Util::writeLog('Encryption library', 'Initial encryption failed!', \OCP\Util::FATAL);
+				$util->resetMigrationStatus();
+				\OCP\User::logout();
 			}
 		}
 
-		return true;
-
+		return $result;
 	}
 
 	/**

apps/files_encryption/lib/util.php

@@ -303,7 +303,7 @@ class Util {
 	 * Find all files and their encryption status within a directory
 	 * @param string $directory The path of the parent directory to search
 	 * @param bool $found the founded files if called again
-	 * @return mixed false if 0 found, array on success. Keys: name, path
+	 * @return array keys: plain, encrypted, legacy, broken
 	 * @note $directory needs to be a path relative to OC data dir. e.g.
 	 *       /admin/files NOT /backup OR /home/www/oc/data/admin/files
 	 */
@@ -322,10 +322,7 @@ class Util {
 			);
 		}
 
-		if (
-			$this->view->is_dir($directory)
-			&& $handle = $this->view->opendir($directory)
-		) {
+		if ($this->view->is_dir($directory) && $handle = $this->view->opendir($directory)){
 			if (is_resource($handle)) {
 				while (false !== ($file = readdir($handle))) {
 
@@ -390,34 +387,16 @@ class Util {
 									'name' => $file,
 									'path' => $relPath
 								);
-
 							}
-
 						}
-
 					}
-
+				}
 			}
 		}
 
 		\OC_FileProxy::$enabled = true;
 
-			if (empty($found)) {
-
-				return false;
-
-			} else {
-
 		return $found;
-
-			}
-
-		}
-
-		\OC_FileProxy::$enabled = true;
-
-		return false;
-
 	}
 
 	/**
@@ -571,28 +550,6 @@ class Util {
 		return $result;
 	}
 
-
-	/**
-	 * @param string $path
-	 * @return bool
-	 */
-	public function isSharedPath($path) {
-
-		$trimmed = ltrim($path, '/');
-		$split = explode('/', $trimmed);
-
-		if (isset($split[2]) && $split[2] === 'Shared') {
-
-			return true;
-
-		} else {
-
-			return false;
-
-		}
-
-	}
-
 	/**
 	 * encrypt versions from given file
 	 * @param array $filelist list of encrypted files, relative to data/user/files
@@ -808,9 +765,9 @@ class Util {
 	 */
 	public function encryptAll($dirPath, $legacyPassphrase = null, $newPassphrase = null) {
 
-		$found = $this->findEncFiles($dirPath);
+		$result = true;
 
-		if ($found) {
+		$found = $this->findEncFiles($dirPath);
 
 		// Disable proxy to prevent file being encrypted twice
 		\OC_FileProxy::$enabled = false;
@@ -841,7 +798,7 @@ class Util {
 			// Open enc file handle for binary writing, with same filename as original plain file
 			$encHandle = fopen('crypt://' . $rawPath . '.part', 'wb');
 
-				if (is_resource($encHandle)) {
+			if (is_resource($encHandle) && is_resource($plainHandle)) {
 				// Move plain file to a temporary location
 				$size = stream_copy_to_stream($plainHandle, $encHandle);
 
@@ -869,14 +826,14 @@ class Util {
 				));
 
 				$encryptedFiles[] = $relPath;
+			} else {
+				\OCP\Util::writeLog('files_encryption', 'initial encryption: could not encrypt ' . $rawPath, \OCP\Util::FATAL);
+				$result = false;
 			}
 		}
 
 		// Encrypt legacy encrypted files
-			if (
-				!empty($legacyPassphrase)
-				&& !empty($newPassphrase)
-			) {
+		if (!empty($legacyPassphrase) && !empty($newPassphrase)) {
 
 			foreach ($found['legacy'] as $legacyFile) {
 
@@ -901,6 +858,9 @@ class Util {
 
 					// close stream
 					fclose($encHandle);
+				} else {
+					\OCP\Util::writeLog('files_encryption', 'initial encryption: could not encrypt legacy file ' . $rawPath, \OCP\Util::FATAL);
+					$result = false;
 				}
 
 				// disable proxy to prevent file being encrypted twice
@@ -914,15 +874,10 @@ class Util {
 			\OC_App::enable('files_versions');
 		}
 
-			$this->encryptVersions($encryptedFiles);
+		$result = $result && $this->encryptVersions($encryptedFiles);
 
-			// If files were found, return true
-			return true;
-		} else {
+		return $result;
 
-			// If no files were found, return false
-			return false;
-		}
 	}
 
 	/**

apps/files_encryption/tests/helper.php

@@ -18,15 +18,20 @@ use OCA\Encryption;
 class Test_Encryption_Helper extends \PHPUnit_Framework_TestCase {
 
 	const TEST_ENCRYPTION_HELPER_USER1 = "test-helper-user1";
+	const TEST_ENCRYPTION_HELPER_USER2 = "test-helper-user2";
 
 	public static function setUpBeforeClass() {
 		// create test user
+		\Test_Encryption_Util::loginHelper(\Test_Encryption_Helper::TEST_ENCRYPTION_HELPER_USER2, true);
 		\Test_Encryption_Util::loginHelper(\Test_Encryption_Helper::TEST_ENCRYPTION_HELPER_USER1, true);
 	}
 
 	public static function tearDownAfterClass() {
 		// cleanup test user
 		\OC_User::deleteUser(\Test_Encryption_Helper::TEST_ENCRYPTION_HELPER_USER1);
+		\OC_User::deleteUser(\Test_Encryption_Helper::TEST_ENCRYPTION_HELPER_USER2);
+		\OC_Hook::clear();
+		\OC_FileProxy::clearProxies();
 	}
 
 	/**
@@ -81,9 +86,11 @@ class Test_Encryption_Helper extends \PHPUnit_Framework_TestCase {
 
 		$path1 = "/" . self::TEST_ENCRYPTION_HELPER_USER1 . "/files/foo/bar.txt";
 		$path2 = "/" . self::TEST_ENCRYPTION_HELPER_USER1 . "/cache/foo/bar.txt";
-		$path3 = "/" . self::TEST_ENCRYPTION_HELPER_USER1 . "/thumbnails/foo";
+		$path3 = "/" . self::TEST_ENCRYPTION_HELPER_USER2 . "/thumbnails/foo";
 		$path4 ="/" . "/" . self::TEST_ENCRYPTION_HELPER_USER1;
 
+		\Test_Encryption_Util::loginHelper(self::TEST_ENCRYPTION_HELPER_USER1);
+
 		// if we are logged-in every path should return the currently logged-in user
 		$this->assertEquals(self::TEST_ENCRYPTION_HELPER_USER1, Encryption\Helper::getUser($path3));
 

apps/files_encryption/tests/util.php

@@ -306,18 +306,6 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase {
 		$this->view->unlink($this->userId . '/files/' . $filename);
 	}
 
-	/**
-	 * @medium
-	 */
-	function testIsSharedPath() {
-		$sharedPath = '/user1/files/Shared/test';
-		$path = '/user1/files/test';
-
-		$this->assertTrue($this->util->isSharedPath($sharedPath));
-
-		$this->assertFalse($this->util->isSharedPath($path));
-	}
-
 	function testEncryptAll() {
 
 		$filename = "/encryptAll" . uniqid() . ".txt";