Merge pull request #17701 from owncloud/fix_repeated_migration
don't move keys if the key where already moved in a previous migration run
This commit is contained in:
commit
89d6439445
6 changed files with 124 additions and 25 deletions
|
@ -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));
|
||||
|
|
|
@ -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)) {
|
||||
|
|
|
@ -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.');
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -279,23 +299,49 @@ 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;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
@ -100,6 +105,17 @@ class MigrationTest extends \Test\TestCase {
|
|||
$this->view->file_put_contents($uid . '/files_encryption/keys/folder2/file.2.1/fileKey' , 'data');
|
||||
}
|
||||
|
||||
protected function createDummyFiles($uid) {
|
||||
$this->view->mkdir($uid . '/files/folder1/folder2/folder3/file3');
|
||||
$this->view->mkdir($uid . '/files/folder1/folder2/file2');
|
||||
$this->view->mkdir($uid . '/files/folder1/file.1');
|
||||
$this->view->mkdir($uid . '/files/folder2/file.2.1');
|
||||
$this->view->file_put_contents($uid . '/files/folder1/folder2/folder3/file3/fileKey' , 'data');
|
||||
$this->view->file_put_contents($uid . '/files/folder1/folder2/file2/fileKey' , 'data');
|
||||
$this->view->file_put_contents($uid . '/files/folder1/file.1/fileKey' , 'data');
|
||||
$this->view->file_put_contents($uid . '/files/folder2/file.2.1/fileKey' , 'data');
|
||||
}
|
||||
|
||||
protected function createDummyFilesInTrash($uid) {
|
||||
$this->view->mkdir($uid . '/files_trashbin/keys/file1.d5457864');
|
||||
$this->view->mkdir($uid . '/files_trashbin/keys/folder1.d7437648723/file2');
|
||||
|
@ -109,6 +125,11 @@ class MigrationTest extends \Test\TestCase {
|
|||
|
||||
$this->view->file_put_contents($uid . '/files_trashbin/keys/file1.d5457864/fileKey' , 'data');
|
||||
$this->view->file_put_contents($uid . '/files_trashbin/keys/folder1.d7437648723/file2/fileKey' , 'data');
|
||||
|
||||
// create the files itself
|
||||
$this->view->mkdir($uid . '/files_trashbin/folder1.d7437648723');
|
||||
$this->view->file_put_contents($uid . '/files_trashbin/file1.d5457864' , 'data');
|
||||
$this->view->file_put_contents($uid . '/files_trashbin/folder1.d7437648723/file2' , 'data');
|
||||
}
|
||||
|
||||
protected function createDummySystemWideKeys() {
|
||||
|
@ -118,7 +139,6 @@ class MigrationTest extends \Test\TestCase {
|
|||
$this->view->file_put_contents('files_encryption/systemwide_2.privateKey', 'data');
|
||||
$this->view->file_put_contents('files_encryption/public_keys/systemwide_1.publicKey', 'data');
|
||||
$this->view->file_put_contents('files_encryption/public_keys/systemwide_2.publicKey', 'data');
|
||||
|
||||
}
|
||||
|
||||
public function testMigrateToNewFolderStructure() {
|
||||
|
@ -134,6 +154,10 @@ class MigrationTest extends \Test\TestCase {
|
|||
$this->createDummyFileKeys(self::TEST_ENCRYPTION_MIGRATION_USER2);
|
||||
$this->createDummyFileKeys(self::TEST_ENCRYPTION_MIGRATION_USER3);
|
||||
|
||||
$this->createDummyFiles(self::TEST_ENCRYPTION_MIGRATION_USER1);
|
||||
$this->createDummyFiles(self::TEST_ENCRYPTION_MIGRATION_USER2);
|
||||
$this->createDummyFiles(self::TEST_ENCRYPTION_MIGRATION_USER3);
|
||||
|
||||
$this->createDummyFilesInTrash(self::TEST_ENCRYPTION_MIGRATION_USER2);
|
||||
|
||||
// no user for system wide mount points
|
||||
|
@ -142,7 +166,22 @@ class MigrationTest extends \Test\TestCase {
|
|||
|
||||
$this->createDummySystemWideKeys();
|
||||
|
||||
$m = new Migration(\OC::$server->getConfig(), new \OC\Files\View(), \OC::$server->getDatabaseConnection());
|
||||
$m = $this->getMockBuilder('OCA\Encryption\Migration')
|
||||
->setConstructorArgs(
|
||||
[
|
||||
\OC::$server->getConfig(),
|
||||
new \OC\Files\View(),
|
||||
\OC::$server->getDatabaseConnection(),
|
||||
$this->logger
|
||||
]
|
||||
)->setMethods(['getSystemMountPoints'])->getMock();
|
||||
|
||||
$m->expects($this->any())->method('getSystemMountPoints')
|
||||
->willReturn([['mountpoint' => 'folder1'], ['mountpoint' => 'folder2']]);
|
||||
|
||||
//$m = new Migration(\OC::$server->getConfig(), new \OC\Files\View(), \OC::$server->getDatabaseConnection(), $this->logger);
|
||||
$m->reorganizeFolderStructure();
|
||||
// even if it runs twice folder should always move only once
|
||||
$m->reorganizeFolderStructure();
|
||||
|
||||
$this->assertTrue(
|
||||
|
@ -267,7 +306,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 +325,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 +388,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
|
||||
|
|
|
@ -79,7 +79,8 @@ class Application extends App {
|
|||
$c->query('Config'),
|
||||
$c->query('DatabaseConnection'),
|
||||
$c->query('UserManager'),
|
||||
new View()
|
||||
new View(),
|
||||
$c->query('Logger')
|
||||
);
|
||||
});
|
||||
$container->registerService('AppSettingsController', function(IContainer $c) {
|
||||
|
|
|
@ -25,6 +25,7 @@ use OC\Files\View;
|
|||
use OCA\Encryption\Migration;
|
||||
use OCP\IL10N;
|
||||
use OCP\AppFramework\Controller;
|
||||
use OCP\ILogger;
|
||||
use OCP\IRequest;
|
||||
use OCP\IConfig;
|
||||
use OC\DB\Connection;
|
||||
|
@ -50,6 +51,9 @@ class EncryptionController extends Controller {
|
|||
/** @var View */
|
||||
private $view;
|
||||
|
||||
/** @var ILogger */
|
||||
private $logger;
|
||||
|
||||
/**
|
||||
* @param string $appName
|
||||
* @param IRequest $request
|
||||
|
@ -58,6 +62,7 @@ class EncryptionController extends Controller {
|
|||
* @param \OC\DB\Connection $connection
|
||||
* @param IUserManager $userManager
|
||||
* @param View $view
|
||||
* @param ILogger $logger
|
||||
*/
|
||||
public function __construct($appName,
|
||||
IRequest $request,
|
||||
|
@ -65,7 +70,8 @@ class EncryptionController extends Controller {
|
|||
IConfig $config,
|
||||
Connection $connection,
|
||||
IUserManager $userManager,
|
||||
View $view) {
|
||||
View $view,
|
||||
ILogger $logger) {
|
||||
parent::__construct($appName, $request);
|
||||
$this->l10n = $l10n;
|
||||
$this->config = $config;
|
||||
|
@ -85,7 +91,7 @@ class EncryptionController extends Controller {
|
|||
|
||||
try {
|
||||
|
||||
$migration = new Migration($this->config, $this->view, $this->connection);
|
||||
$migration = new Migration($this->config, $this->view, $this->connection, $this->logger);
|
||||
$migration->reorganizeSystemFolderStructure();
|
||||
$migration->updateDB();
|
||||
|
||||
|
|
Loading…
Reference in a new issue