don't move keys if the key where already moved in a previous migration run

4 files changed, 79 insertions, 21 deletions

diff --git a/apps/encryption/appinfo/register_command.php b/apps/encryption/appinfo/register_command.php
index f727fdf9d70..4fdf7ecec38 100644
--- a/apps/encryption/appinfo/register_command.php
+++ b/apps/encryption/appinfo/register_command.php
@@ -26,4 +26,5 @@ $userManager = OC::$server->getUserManager();
 $view = new \OC\Files\View();
 $config = \OC::$server->getConfig();
 $connection = \OC::$server->getDatabaseConnection();
-$application->add(new MigrateKeys($userManager, $view, $connection, $config));
+$logger = \OC::$server->getLogger();
+$application->add(new MigrateKeys($userManager, $view, $connection, $config, $logger));
diff --git a/apps/encryption/command/migratekeys.php b/apps/encryption/command/migratekeys.php
index d0fc1573061..7e320102172 100644
--- a/apps/encryption/command/migratekeys.php
+++ b/apps/encryption/command/migratekeys.php
@@ -27,6 +27,7 @@ use OC\Files\View;
 use OC\User\Manager;
 use OCA\Encryption\Migration;
 use OCP\IConfig;
+use OCP\ILogger;
 use OCP\IUserBackend;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
@@ -44,22 +45,27 @@ class MigrateKeys extends Command {
 	private $connection;
 	/** @var IConfig */
 	private $config;
+	/** @var  ILogger */
+	private $logger;
 
 	/**
 	 * @param Manager $userManager
 	 * @param View $view
 	 * @param Connection $connection
 	 * @param IConfig $config
+	 * @param ILogger $logger
 	 */
 	public function __construct(Manager $userManager,
 								View $view,
 								Connection $connection,
-								IConfig $config) {
+								IConfig $config,
+								ILogger $logger) {
 
 		$this->userManager = $userManager;
 		$this->view = $view;
 		$this->connection = $connection;
 		$this->config = $config;
+		$this->logger = $logger;
 		parent::__construct();
 	}
 
@@ -77,7 +83,7 @@ class MigrateKeys extends Command {
 	protected function execute(InputInterface $input, OutputInterface $output) {
 
 		// perform system reorganization
-		$migration = new Migration($this->config, $this->view, $this->connection);
+		$migration = new Migration($this->config, $this->view, $this->connection, $this->logger);
 
 		$users = $input->getArgument('user_id');
 		if (!empty($users)) {
diff --git a/apps/encryption/lib/migration.php b/apps/encryption/lib/migration.php
index 26e2a143f69..0903587e879 100644
--- a/apps/encryption/lib/migration.php
+++ b/apps/encryption/lib/migration.php
@@ -26,6 +26,7 @@ namespace OCA\Encryption;
 use OC\DB\Connection;
 use OC\Files\View;
 use OCP\IConfig;
+use OCP\ILogger;
 
 class Migration {
 
@@ -37,17 +38,22 @@ class Migration {
 	/** @var IConfig */
 	private $config;
 
+	/** @var  ILogger */
+	private $logger;
+
 	/**
 	 * @param IConfig $config
 	 * @param View $view
 	 * @param Connection $connection
+	 * @param ILogger $logger
 	 */
-	public function __construct(IConfig $config, View $view, Connection $connection) {
+	public function __construct(IConfig $config, View $view, Connection $connection, ILogger $logger) {
 		$this->view = $view;
 		$this->view->getUpdater()->disable();
 		$this->connection = $connection;
 		$this->moduleId = \OCA\Encryption\Crypto\Encryption::ID;
 		$this->config = $config;
+		$this->logger = $logger;
 	}
 
 	public function finalCleanUp() {
@@ -234,9 +240,10 @@ class Migration {
 	private function renameUsersPrivateKey($user) {
 		$oldPrivateKey = $user . '/files_encryption/' . $user . '.privateKey';
 		$newPrivateKey = $user . '/files_encryption/' . $this->moduleId . '/' . $user . '.privateKey';
-		$this->createPathForKeys(dirname($newPrivateKey));
-
-		$this->view->rename($oldPrivateKey, $newPrivateKey);
+		if ($this->view->file_exists($oldPrivateKey)) {
+			$this->createPathForKeys(dirname($newPrivateKey));
+			$this->view->rename($oldPrivateKey, $newPrivateKey);
+		}
 	}
 
 	/**
@@ -247,9 +254,10 @@ class Migration {
 	private function renameUsersPublicKey($user) {
 		$oldPublicKey = '/files_encryption/public_keys/' . $user . '.publicKey';
 		$newPublicKey = $user . '/files_encryption/' . $this->moduleId . '/' . $user . '.publicKey';
-		$this->createPathForKeys(dirname($newPublicKey));
-
-		$this->view->rename($oldPublicKey, $newPublicKey);
+		if ($this->view->file_exists($oldPublicKey)) {
+			$this->createPathForKeys(dirname($newPublicKey));
+			$this->view->rename($oldPublicKey, $newPublicKey);
+		}
 	}
 
 	/**
@@ -261,6 +269,11 @@ class Migration {
 	 */
 	private function renameFileKeys($user, $path, $trash = false) {
 
+		if ($this->view->is_dir($user . '/' . $path) === false) {
+			$this->logger->info('Skip dir /' . $user . '/' . $path . ': does not exist');
+			return;
+		}
+
 		$dh = $this->view->opendir($user . '/' . $path);
 
 		if (is_resource($dh)) {
@@ -270,8 +283,15 @@ class Migration {
 						$this->renameFileKeys($user, $path . '/' . $file, $trash);
 					} else {
 						$target = $this->getTargetDir($user, $path, $file, $trash);
-						$this->createPathForKeys(dirname($target));
-						$this->view->rename($user . '/' . $path . '/' . $file, $target);
+						if ($target) {
+							$this->createPathForKeys(dirname($target));
+							$this->view->rename($user . '/' . $path . '/' . $file, $target);
+						} else {
+							$this->logger->warning(
+								'did not move key "' . $file
+								. '" could not find the corresponding file in /data/' . $user . '/files.'
+							. 'Most likely the key was already moved in a previous migration run and is already on the right place.');
+						}
 					}
 				}
 			}
@@ -280,22 +300,48 @@ class Migration {
 	}
 
 	/**
+	 * get system mount points
+	 * wrap static method so that it can be mocked for testing
+	 *
+	 * @return array
+	 */
+	protected function getSystemMountPoints() {
+		return \OC_Mount_Config::getSystemMountPoints();
+	}
+
+	/**
 	 * generate target directory
 	 *
 	 * @param string $user
-	 * @param string $filePath
+	 * @param string $keyPath
 	 * @param string $filename
 	 * @param bool $trash
 	 * @return string
 	 */
-	private function getTargetDir($user, $filePath, $filename, $trash) {
+	private function getTargetDir($user, $keyPath, $filename, $trash) {
 		if ($trash) {
-			$targetDir = $user . '/files_encryption/keys/files_trashbin/' . substr($filePath, strlen('/files_trashbin/keys/')) . '/' . $this->moduleId . '/' . $filename;
+			$filePath = substr($keyPath, strlen('/files_trashbin/keys/'));
+			$targetDir = $user . '/files_encryption/keys/files_trashbin/' . $filePath . '/' . $this->moduleId . '/' . $filename;
 		} else {
-			$targetDir = $user . '/files_encryption/keys/files/' . substr($filePath, strlen('/files_encryption/keys/')) . '/' . $this->moduleId . '/' . $filename;
+			$filePath = substr($keyPath, strlen('/files_encryption/keys/'));
+			$targetDir = $user . '/files_encryption/keys/files/' . $filePath . '/' . $this->moduleId . '/' . $filename;
 		}
 
-		return $targetDir;
+		if ($user === '') {
+			// for system wide mounts we need to check if the mount point really exists
+			$normalized = trim($filePath, '/');
+			$systemMountPoints = $this->getSystemMountPoints();
+			foreach ($systemMountPoints as $mountPoint) {
+				if (strpos($normalized, $mountPoint['mountpoint']) === 0)
+					return $targetDir;
+			}
+		} else if ($trash === false && $this->view->file_exists('/' . $user. '/files/' . $filePath)) {
+			return $targetDir;
+		} else if ($trash === true && $this->view->file_exists('/' . $user. '/files_trashbin/' . $filePath)) {
+				return $targetDir;
+			}
+
+		return false;
 	}
 
 	/**
diff --git a/apps/encryption/tests/lib/MigrationTest.php b/apps/encryption/tests/lib/MigrationTest.php
index de1e2bd268b..786bce33a2c 100644
--- a/apps/encryption/tests/lib/MigrationTest.php
+++ b/apps/encryption/tests/lib/MigrationTest.php
@@ -24,6 +24,7 @@
 namespace OCA\Encryption\Tests;
 
 use OCA\Encryption\Migration;
+use OCP\ILogger;
 
 class MigrationTest extends \Test\TestCase {
 
@@ -37,6 +38,9 @@ class MigrationTest extends \Test\TestCase {
 	private $recovery_key_id = 'recovery_key_id';
 	private $moduleId;
 
+	/** @var  PHPUnit_Framework_MockObject_MockObject | ILogger */
+	private $logger;
+
 	public static function setUpBeforeClass() {
 		parent::setUpBeforeClass();
 		\OC_User::createUser(self::TEST_ENCRYPTION_MIGRATION_USER1, 'foo');
@@ -53,6 +57,7 @@ class MigrationTest extends \Test\TestCase {
 
 
 	public function setUp() {
+		$this->logger = $this->getMockBuilder('\OCP\ILogger')->disableOriginalConstructor()->getMock();
 		$this->view = new \OC\Files\View();
 		$this->moduleId = \OCA\Encryption\Crypto\Encryption::ID;
 	}
@@ -142,7 +147,7 @@ class MigrationTest extends \Test\TestCase {
 
 		$this->createDummySystemWideKeys();
 
-		$m = new Migration(\OC::$server->getConfig(), new \OC\Files\View(), \OC::$server->getDatabaseConnection());
+		$m = new Migration(\OC::$server->getConfig(), new \OC\Files\View(), \OC::$server->getDatabaseConnection(), $this->logger);
 		$m->reorganizeFolderStructure();
 
 		$this->assertTrue(
@@ -267,7 +272,7 @@ class MigrationTest extends \Test\TestCase {
 	public function testUpdateDB() {
 		$this->prepareDB();
 
-		$m = new Migration(\OC::$server->getConfig(), new \OC\Files\View(), \OC::$server->getDatabaseConnection());
+		$m = new Migration(\OC::$server->getConfig(), new \OC\Files\View(), \OC::$server->getDatabaseConnection(), $this->logger);
 		$m->updateDB();
 
 		$this->verifyDB('`*PREFIX*appconfig`', 'files_encryption', 0);
@@ -286,7 +291,7 @@ class MigrationTest extends \Test\TestCase {
 		$config->setAppValue('encryption', 'publicShareKeyId', 'wrong_share_id');
 		$config->setUserValue(self::TEST_ENCRYPTION_MIGRATION_USER1, 'encryption', 'recoverKeyEnabled', '9');
 
-		$m = new Migration(\OC::$server->getConfig(), new \OC\Files\View(), \OC::$server->getDatabaseConnection());
+		$m = new Migration(\OC::$server->getConfig(), new \OC\Files\View(), \OC::$server->getDatabaseConnection(), $this->logger);
 		$m->updateDB();
 
 		$this->verifyDB('`*PREFIX*appconfig`', 'files_encryption', 0);
@@ -349,7 +354,7 @@ class MigrationTest extends \Test\TestCase {
 	 */
 	public function testUpdateFileCache() {
 		$this->prepareFileCache();
-		$m = new Migration(\OC::$server->getConfig(), new \OC\Files\View(), \OC::$server->getDatabaseConnection());
+		$m = new Migration(\OC::$server->getConfig(), new \OC\Files\View(), \OC::$server->getDatabaseConnection(), $this->logger);
 		self::invokePrivate($m, 'updateFileCache');
 
 		// check results