From df438b22aeb6df8381f0e0e2de07e72403a918a3 Mon Sep 17 00:00:00 2001 From: Wizz Date: Sat, 21 Dec 2019 09:46:01 -0500 Subject: [PATCH] CLI: Fix keyfile from/to parameter collision in merge command Rename 'k' to 'f' because 'k' is already used to specify the key for the target database of the merge * Remove short -f option from keepassxc-cli.1 * Remove -f option from keepassxc-cli merge * Add test cases covering cli options for merge * Add functional test for merge with keys --- share/docs/man/keepassxc-cli.1 | 2 +- src/cli/Merge.cpp | 3 +- tests/CMakeLists.txt | 6 +- tests/TestCli.cpp | 101 +++++++++++++++++++++++++++++++++ tests/TestCli.h | 1 + 5 files changed, 107 insertions(+), 6 deletions(-) diff --git a/share/docs/man/keepassxc-cli.1 b/share/docs/man/keepassxc-cli.1 index bcc97efaea..2be6b198ac 100644 --- a/share/docs/man/keepassxc-cli.1 +++ b/share/docs/man/keepassxc-cli.1 @@ -117,7 +117,7 @@ Displays the program version. .IP "-d, --dry-run " Prints the changes detected by the merge operation without making any changes to the database. -.IP "-f, --key-file-from " +.IP "--key-file-from " Sets the path of the key file for the second database. .IP "--no-password-from" diff --git a/src/cli/Merge.cpp b/src/cli/Merge.cpp index f02794a4bb..5855eff469 100644 --- a/src/cli/Merge.cpp +++ b/src/cli/Merge.cpp @@ -30,8 +30,7 @@ const QCommandLineOption Merge::SameCredentialsOption = QObject::tr("Use the same credentials for both database files.")); const QCommandLineOption Merge::KeyFileFromOption = - QCommandLineOption(QStringList() << "k" - << "key-file-from", + QCommandLineOption(QStringList() << "key-file-from", QObject::tr("Key file of the database to merge from."), QObject::tr("path")); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9aac1b7d8e..1c0e5f7ed6 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -164,7 +164,7 @@ endif() if(WITH_XC_CRYPTO_SSH) add_unit_test(NAME testopensshkey SOURCES TestOpenSSHKey.cpp - LIBS ${TEST_LIBRARIES}) + LIBS ${TEST_LIBRARIES}) endif() add_unit_test(NAME testentry SOURCES TestEntry.cpp @@ -192,7 +192,7 @@ add_unit_test(NAME testcsvparser SOURCES TestCsvParser.cpp LIBS ${TEST_LIBRARIES}) add_unit_test(NAME testrandomgenerator SOURCES TestRandomGenerator.cpp - LIBS testsupport ${TEST_LIBRARIES}) + LIBS testsupport ${TEST_LIBRARIES}) add_unit_test(NAME testentrysearcher SOURCES TestEntrySearcher.cpp LIBS ${TEST_LIBRARIES}) @@ -206,7 +206,7 @@ add_unit_test(NAME testykchallengeresponsekey if(WITH_XC_KEESHARE) add_unit_test(NAME testsharing SOURCES TestSharing.cpp - LIBS testsupport ${TEST_LIBRARIES}) + LIBS testsupport ${TEST_LIBRARIES}) endif() add_unit_test(NAME testdatabase SOURCES TestDatabase.cpp diff --git a/tests/TestCli.cpp b/tests/TestCli.cpp index 9a2756eac2..076f7f74ed 100644 --- a/tests/TestCli.cpp +++ b/tests/TestCli.cpp @@ -1422,6 +1422,107 @@ void TestCli::testMerge() QCOMPARE(m_stdoutFile->readAll(), QByteArray("")); } +void TestCli::testMergeWithKeys() +{ + Create createCmd; + QVERIFY(!createCmd.name.isEmpty()); + QVERIFY(createCmd.getDescriptionLine().contains(createCmd.name)); + + Merge mergeCmd; + QVERIFY(!mergeCmd.name.isEmpty()); + QVERIFY(mergeCmd.getDescriptionLine().contains(mergeCmd.name)); + + Kdbx4Writer writer; + Kdbx4Reader reader; + + QScopedPointer testDir(new QTemporaryDir()); + + QString sourceDatabaseFilename = testDir->path() + "/testSourceDatabase.kdbx"; + QString sourceKeyfilePath = testDir->path() + "/testSourceKeyfile.txt"; + + QString targetDatabaseFilename = testDir->path() + "/testTargetDatabase.kdbx"; + QString targetKeyfilePath = testDir->path() + "/testTargetKeyfile.txt"; + + qint64 pos = m_stdoutFile->pos(); + + Utils::Test::setNextPassword("a"); + createCmd.execute({"create", sourceDatabaseFilename, "-k", sourceKeyfilePath}); + + Utils::Test::setNextPassword("b"); + createCmd.execute({"create", targetDatabaseFilename, "-k", targetKeyfilePath}); + + Utils::Test::setNextPassword("a"); + auto sourceDatabase = QSharedPointer( + Utils::unlockDatabase(sourceDatabaseFilename, true, sourceKeyfilePath, "", Utils::STDOUT)); + QVERIFY(sourceDatabase); + + Utils::Test::setNextPassword("b"); + auto targetDatabase = QSharedPointer( + Utils::unlockDatabase(targetDatabaseFilename, true, targetKeyfilePath, "", Utils::STDOUT)); + QVERIFY(targetDatabase); + + auto* rootGroup = new Group(); + rootGroup->setName("root"); + rootGroup->setUuid(QUuid::createUuid()); + auto* group = new Group(); + group->setUuid(QUuid::createUuid()); + group->setParent(rootGroup); + group->setName("Internet"); + + auto* entry = new Entry(); + entry->setUuid(QUuid::createUuid()); + entry->setTitle("Some Website"); + entry->setPassword("secretsecretsecret"); + group->addEntry(entry); + + sourceDatabase->setRootGroup(rootGroup); + + auto* otherRootGroup = new Group(); + otherRootGroup->setName("root"); + otherRootGroup->setUuid(QUuid::createUuid()); + auto* otherGroup = new Group(); + otherGroup->setUuid(QUuid::createUuid()); + otherGroup->setParent(otherRootGroup); + otherGroup->setName("Internet"); + + auto* otherEntry = new Entry(); + otherEntry->setUuid(QUuid::createUuid()); + otherEntry->setTitle("Some Website 2"); + otherEntry->setPassword("secretsecretsecret 2"); + otherGroup->addEntry(otherEntry); + + targetDatabase->setRootGroup(otherRootGroup); + + QFile sourceDatabaseFile(sourceDatabaseFilename); + sourceDatabaseFile.open(QIODevice::WriteOnly); + QVERIFY(writer.writeDatabase(&sourceDatabaseFile, sourceDatabase.data())); + sourceDatabaseFile.flush(); + sourceDatabaseFile.close(); + + QFile targetDatabaseFile(targetDatabaseFilename); + targetDatabaseFile.open(QIODevice::WriteOnly); + QVERIFY(writer.writeDatabase(&targetDatabaseFile, targetDatabase.data())); + targetDatabaseFile.flush(); + targetDatabaseFile.close(); + + pos = m_stdoutFile->pos(); + Utils::Test::setNextPassword("b"); + Utils::Test::setNextPassword("a"); + mergeCmd.execute({"merge", + "-k", + targetKeyfilePath, + "--key-file-from", + sourceKeyfilePath, + targetDatabaseFile.fileName(), + sourceDatabaseFile.fileName()}); + + m_stdoutFile->seek(pos); + QList lines = m_stdoutFile->readAll().split('\n'); + QVERIFY(lines.contains(QString("Successfully merged %1 into %2.") + .arg(sourceDatabaseFile.fileName(), targetDatabaseFile.fileName()) + .toUtf8())); +} + void TestCli::testMove() { Move moveCmd; diff --git a/tests/TestCli.h b/tests/TestCli.h index bd0f9fc3fe..4947ee472b 100644 --- a/tests/TestCli.h +++ b/tests/TestCli.h @@ -66,6 +66,7 @@ private slots: void testList(); void testLocate(); void testMerge(); + void testMergeWithKeys(); void testMove(); void testOpen(); void testRemove();