Merge pull request #17473 from owncloud/enc_fix_migration

more secure way to update the database

2 files changed, 82 insertions, 14 deletions

diff --git a/apps/encryption/lib/migration.php b/apps/encryption/lib/migration.php
index 98fc5be777a..b5d5dc26568 100644
--- a/apps/encryption/lib/migration.php
+++ b/apps/encryption/lib/migration.php
@@ -143,22 +143,32 @@ class Migration {
 		$this->config->deleteAppValue('files_encryption', 'types');
 		$this->config->deleteAppValue('files_encryption', 'enabled');
 
+		$oldAppValues = $this->connection->createQueryBuilder();
+		$oldAppValues->select('*')
+			->from('`*PREFIX*appconfig`')
+			->where($oldAppValues->expr()->eq('`appid`', ':appid'))
+			->setParameter('appid', 'files_encryption');
+		$appSettings = $oldAppValues->execute();
+
+		while ($row = $appSettings->fetch()) {
+			// 'installed_version' gets deleted at the end of the migration process
+			if ($row['configkey'] !== 'installed_version' ) {
+				$this->config->setAppValue('encryption', $row['configkey'], $row['configvalue']);
+				$this->config->deleteAppValue('files_encryption', $row['configkey']);
+			}
+		}
 
-		$query = $this->connection->createQueryBuilder();
-		$query->update('`*PREFIX*appconfig`')
-			->set('`appid`', ':newappid')
-			->where($query->expr()->eq('`appid`', ':oldappid'))
-			->setParameter('oldappid', 'files_encryption')
-			->setParameter('newappid', 'encryption');
-		$query->execute();
+		$oldPreferences = $this->connection->createQueryBuilder();
+		$oldPreferences->select('*')
+			->from('`*PREFIX*preferences`')
+			->where($oldPreferences->expr()->eq('`appid`', ':appid'))
+			->setParameter('appid', 'files_encryption');
+		$preferenceSettings = $oldPreferences->execute();
 
-		$query = $this->connection->createQueryBuilder();
-		$query->update('`*PREFIX*preferences`')
-			->set('`appid`', ':newappid')
-			->where($query->expr()->eq('`appid`', ':oldappid'))
-			->setParameter('oldappid', 'files_encryption')
-			->setParameter('newappid', 'encryption');
-		$query->execute();
+		while ($row = $preferenceSettings->fetch()) {
+			$this->config->setUserValue($row['userid'], 'encryption', $row['configkey'], $row['configvalue']);
+			$this->config->deleteUserValue($row['userid'], 'files_encryption', $row['configkey']);
+		}
 	}
 
 	/**
diff --git a/apps/encryption/tests/lib/MigrationTest.php b/apps/encryption/tests/lib/MigrationTest.php
index c07a4539e98..de1e2bd268b 100644
--- a/apps/encryption/tests/lib/MigrationTest.php
+++ b/apps/encryption/tests/lib/MigrationTest.php
@@ -242,6 +242,12 @@ class MigrationTest extends \Test\TestCase {
 		$config->setAppValue('files_encryption', 'recoveryAdminEnabled', '1');
 		$config->setUserValue(self::TEST_ENCRYPTION_MIGRATION_USER1, 'files_encryption', 'recoverKeyEnabled', '1');
 
+		//$this->invokePrivate($config, 'cache', [[]]);
+		$cache = $this->invokePrivate(\OC::$server->getAppConfig(), 'cache');
+		unset($cache['encryption']);
+		unset($cache['files_encryption']);
+		$this->invokePrivate(\OC::$server->getAppConfig(), 'cache', [$cache]);
+
 		// delete default values set by the encryption app during initialization
 
 		/** @var \OC\DB\Connection $connection */
@@ -271,6 +277,58 @@ class MigrationTest extends \Test\TestCase {
 
 	}
 
+	/**
+	 * test update db if the db already contain some existing new values
+	 */
+	public function testUpdateDBExistingNewConfig() {
+		$this->prepareDB();
+		$config = \OC::$server->getConfig();
+		$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->updateDB();
+
+		$this->verifyDB('`*PREFIX*appconfig`', 'files_encryption', 0);
+		$this->verifyDB('`*PREFIX*preferences`', 'files_encryption', 0);
+		$this->verifyDB('`*PREFIX*appconfig`', 'encryption', 3);
+		$this->verifyDB('`*PREFIX*preferences`', 'encryption', 1);
+
+		// check if the existing values where overwritten correctly
+		/** @var \OC\DB\Connection $connection */
+		$connection = \OC::$server->getDatabaseConnection();
+		$query = $connection->createQueryBuilder();
+		$query->select('`configvalue`')
+			->from('`*PREFIX*appconfig`')
+			->where($query->expr()->andX(
+				$query->expr()->eq('`appid`', ':appid'),
+				$query->expr()->eq('`configkey`', ':configkey')
+			))
+			->setParameter('appid', 'encryption')
+			->setParameter('configkey', 'publicShareKeyId');
+		$result = $query->execute();
+		$value = $result->fetch();
+		$this->assertTrue(isset($value['configvalue']));
+		$this->assertSame('share_id', $value['configvalue']);
+
+		$query = $connection->createQueryBuilder();
+		$query->select('`configvalue`')
+			->from('`*PREFIX*preferences`')
+			->where($query->expr()->andX(
+				$query->expr()->eq('`appid`', ':appid'),
+				$query->expr()->eq('`configkey`', ':configkey'),
+				$query->expr()->eq('`userid`', ':userid')
+			))
+			->setParameter('appid', 'encryption')
+			->setParameter('configkey', 'recoverKeyEnabled')
+			->setParameter('userid', self::TEST_ENCRYPTION_MIGRATION_USER1);
+		$result = $query->execute();
+		$value = $result->fetch();
+		$this->assertTrue(isset($value['configvalue']));
+		$this->assertSame('1', $value['configvalue']);
+
+	}
+
 	public function verifyDB($table, $appid, $expected) {
 		/** @var \OC\DB\Connection $connection */
 		$connection = \OC::$server->getDatabaseConnection();