From 9f8943c89bfb4cfc1d6729eaed00b6db97391206 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Mon, 18 Dec 2017 02:44:12 +1100 Subject: [PATCH] keepassxc-cli show: resolve references in output (#1280) * core: database: make UUID searching case-insensitive 4c4d8a5e848c ("Implement search for reference placeholder based on fields other than ID") changed the semantics of searching-by-reference in KeePassXC. Unforuntately it contained a bug where it implicitly became case-sensitive to UUIDs, which broke existing databases that used references (especially since the default reference format uses a different case to the UUID used while searching). The tests didn't catch this because ->toHex() preserves the case that it was provided, they have been updated to check that UUIDs are case insensitive. * cli: show: resolve references in output Previously, `keepassxc-cli show` would not resolve references. This would make it quite hard to script around its output (since there's not interface to resolve references manually either). Fix this by using resolveMultiplePlaceholders as with all other users of ->password() and related entry fields. Fixes: keepassxreboot/keepassxc#1260 * tests: entry: add tests for ref-cloned entries This ensures that the most "intuitive" current usage of references (through the clone feature of the GUI) remains self-consistent and always produces the correct results. In addition, explicitly test that case insensitivity works as expected. These should avoid similar regressions in reference handling in the future. * http: resolve references in AccessControlDialog The access control dialog previously would not show the "real" username or "real" title when asking for permission to give access to entries. Fix this by resolving it, as we do in many other places. Fixes: keepassxreboot/keepassxc#1269 Signed-off-by: Aleksa Sarai --- src/cli/Show.cpp | 2 +- src/core/Database.cpp | 2 +- src/http/AccessControlDialog.cpp | 4 +- tests/TestEntry.cpp | 93 +++++++++++++++++++++++++++++++- tests/TestEntry.h | 1 + 5 files changed, 97 insertions(+), 5 deletions(-) diff --git a/src/cli/Show.cpp b/src/cli/Show.cpp index 43a9a837..66225c56 100644 --- a/src/cli/Show.cpp +++ b/src/cli/Show.cpp @@ -102,7 +102,7 @@ int Show::showEntry(Database* database, QStringList attributes, QString entryPat if (showAttributeNames) { outputTextStream << attribute << ": "; } - outputTextStream << entry->attributes()->value(attribute) << endl; + outputTextStream << entry->resolveMultiplePlaceholders(entry->attributes()->value(attribute)) << endl; } return sawUnknownAttribute ? EXIT_FAILURE : EXIT_SUCCESS; } diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 7ae9308a..cb28ee21 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -150,7 +150,7 @@ Entry* Database::findEntryRecursive(const QString& text, EntryReferenceType refe found = entry->notes() == text; break; case EntryReferenceType::Uuid: - found = entry->uuid().toHex() == text; + found = entry->uuid() == Uuid::fromHex(text); break; case EntryReferenceType::CustomAttributes: found = entry->attributes()->containsValue(text); diff --git a/src/http/AccessControlDialog.cpp b/src/http/AccessControlDialog.cpp index ef02215a..30ab3f21 100644 --- a/src/http/AccessControlDialog.cpp +++ b/src/http/AccessControlDialog.cpp @@ -44,7 +44,9 @@ void AccessControlDialog::setUrl(const QString &url) void AccessControlDialog::setItems(const QList &items) { for (Entry* entry: items) { - ui->itemsList->addItem(entry->title() + " - " + entry->username()); + QString title = entry->resolveMultiplePlaceholders(entry->title()); + QString username = entry->resolveMultiplePlaceholders(entry->username()); + ui->itemsList->addItem(title + " - " + username); } } diff --git a/tests/TestEntry.cpp b/tests/TestEntry.cpp index bb117645..47082d12 100644 --- a/tests/TestEntry.cpp +++ b/tests/TestEntry.cpp @@ -337,12 +337,23 @@ void TestEntry::testResolveReferencePlaceholders() QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:A@A:%1}").arg(entry2->url())), entry2->url()); QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:N@N:%1}").arg(entry2->notes())), entry2->notes()); - QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributeTitle")); QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributeTitle")); QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:U@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributeUsername")); QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:P@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributePassword")); QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:A@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributeUrl")); QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:N@I:%1}").arg(entry3->uuid().toHex())), entry3->attributes()->value("AttributeNotes")); + + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:T@I:%1}").arg(entry3->uuid().toHex().toUpper())), entry3->attributes()->value("AttributeTitle")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:U@I:%1}").arg(entry3->uuid().toHex().toUpper())), entry3->attributes()->value("AttributeUsername")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:P@I:%1}").arg(entry3->uuid().toHex().toUpper())), entry3->attributes()->value("AttributePassword")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:A@I:%1}").arg(entry3->uuid().toHex().toUpper())), entry3->attributes()->value("AttributeUrl")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:N@I:%1}").arg(entry3->uuid().toHex().toUpper())), entry3->attributes()->value("AttributeNotes")); + + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:t@i:%1}").arg(entry3->uuid().toHex().toLower())), entry3->attributes()->value("AttributeTitle")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:u@i:%1}").arg(entry3->uuid().toHex().toLower())), entry3->attributes()->value("AttributeUsername")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:p@i:%1}").arg(entry3->uuid().toHex().toLower())), entry3->attributes()->value("AttributePassword")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:a@i:%1}").arg(entry3->uuid().toHex().toLower())), entry3->attributes()->value("AttributeUrl")); + QCOMPARE(tstEntry->resolveMultiplePlaceholders(QString("{REF:n@i:%1}").arg(entry3->uuid().toHex().toLower())), entry3->attributes()->value("AttributeNotes")); } void TestEntry::testResolveNonIdPlaceholdersToUuid() @@ -409,8 +420,86 @@ void TestEntry::testResolveNonIdPlaceholdersToUuid() newEntry.setGroup(root); newEntry.setNotes(newEntryNotesRaw); - const auto newEntryNotesResolved = + const auto newEntryNotesResolved = newEntry.resolveMultiplePlaceholders(newEntry.notes()); QCOMPARE(newEntryNotesResolved, QString(referencedEntry->uuid().toHex())); } } + +void TestEntry::testResolveClonedEntry() +{ + Database db; + Group* root = db.rootGroup(); + + Entry* original = new Entry(); + original->setGroup(root); + original->setUuid(Uuid::random()); + original->setTitle("Title"); + original->setUsername("SomeUsername"); + original->setPassword("SomePassword"); + + QCOMPARE(original->resolveMultiplePlaceholders(original->username()), original->username()); + QCOMPARE(original->resolveMultiplePlaceholders(original->password()), original->password()); + + // Top-level clones. + Entry* clone1 = original->clone(Entry::CloneNewUuid); + clone1->setGroup(root); + Entry* clone2 = original->clone(Entry::CloneUserAsRef | Entry::CloneNewUuid); + clone2->setGroup(root); + Entry* clone3 = original->clone(Entry::ClonePassAsRef | Entry::CloneNewUuid); + clone3->setGroup(root); + Entry* clone4 = original->clone(Entry::CloneUserAsRef | Entry::ClonePassAsRef | Entry::CloneNewUuid); + clone4->setGroup(root); + + QCOMPARE(clone1->resolveMultiplePlaceholders(clone1->username()), original->username()); + QCOMPARE(clone1->resolveMultiplePlaceholders(clone1->password()), original->password()); + QCOMPARE(clone2->resolveMultiplePlaceholders(clone2->username()), original->username()); + QCOMPARE(clone2->resolveMultiplePlaceholders(clone2->password()), original->password()); + QCOMPARE(clone3->resolveMultiplePlaceholders(clone3->username()), original->username()); + QCOMPARE(clone3->resolveMultiplePlaceholders(clone3->password()), original->password()); + QCOMPARE(clone4->resolveMultiplePlaceholders(clone4->username()), original->username()); + QCOMPARE(clone4->resolveMultiplePlaceholders(clone4->password()), original->password()); + + // Second-level clones. + Entry* cclone1 = clone4->clone(Entry::CloneNewUuid); + cclone1->setGroup(root); + Entry* cclone2 = clone4->clone(Entry::CloneUserAsRef | Entry::CloneNewUuid); + cclone2->setGroup(root); + Entry* cclone3 = clone4->clone(Entry::ClonePassAsRef | Entry::CloneNewUuid); + cclone3->setGroup(root); + Entry* cclone4 = clone4->clone(Entry::CloneUserAsRef | Entry::ClonePassAsRef | Entry::CloneNewUuid); + cclone4->setGroup(root); + + QCOMPARE(cclone1->resolveMultiplePlaceholders(cclone1->username()), original->username()); + QCOMPARE(cclone1->resolveMultiplePlaceholders(cclone1->password()), original->password()); + QCOMPARE(cclone2->resolveMultiplePlaceholders(cclone2->username()), original->username()); + QCOMPARE(cclone2->resolveMultiplePlaceholders(cclone2->password()), original->password()); + QCOMPARE(cclone3->resolveMultiplePlaceholders(cclone3->username()), original->username()); + QCOMPARE(cclone3->resolveMultiplePlaceholders(cclone3->password()), original->password()); + QCOMPARE(cclone4->resolveMultiplePlaceholders(cclone4->username()), original->username()); + QCOMPARE(cclone4->resolveMultiplePlaceholders(cclone4->password()), original->password()); + + // Change the original's attributes and make sure that the changes are tracked. + QString oldUsername = original->username(); + QString oldPassword = original->password(); + original->setUsername("DifferentUsername"); + original->setPassword("DifferentPassword"); + + QCOMPARE(clone1->resolveMultiplePlaceholders(clone1->username()), oldUsername); + QCOMPARE(clone1->resolveMultiplePlaceholders(clone1->password()), oldPassword); + QCOMPARE(clone2->resolveMultiplePlaceholders(clone2->username()), original->username()); + QCOMPARE(clone2->resolveMultiplePlaceholders(clone2->password()), oldPassword); + QCOMPARE(clone3->resolveMultiplePlaceholders(clone3->username()), oldUsername); + QCOMPARE(clone3->resolveMultiplePlaceholders(clone3->password()), original->password()); + QCOMPARE(clone4->resolveMultiplePlaceholders(clone4->username()), original->username()); + QCOMPARE(clone4->resolveMultiplePlaceholders(clone4->password()), original->password()); + + QCOMPARE(cclone1->resolveMultiplePlaceholders(cclone1->username()), original->username()); + QCOMPARE(cclone1->resolveMultiplePlaceholders(cclone1->password()), original->password()); + QCOMPARE(cclone2->resolveMultiplePlaceholders(cclone2->username()), original->username()); + QCOMPARE(cclone2->resolveMultiplePlaceholders(cclone2->password()), original->password()); + QCOMPARE(cclone3->resolveMultiplePlaceholders(cclone3->username()), original->username()); + QCOMPARE(cclone3->resolveMultiplePlaceholders(cclone3->password()), original->password()); + QCOMPARE(cclone4->resolveMultiplePlaceholders(cclone4->username()), original->username()); + QCOMPARE(cclone4->resolveMultiplePlaceholders(cclone4->password()), original->password()); +} diff --git a/tests/TestEntry.h b/tests/TestEntry.h index 44a371ba..7c235086 100644 --- a/tests/TestEntry.h +++ b/tests/TestEntry.h @@ -36,6 +36,7 @@ private slots: void testResolveRecursivePlaceholders(); void testResolveReferencePlaceholders(); void testResolveNonIdPlaceholdersToUuid(); + void testResolveClonedEntry(); }; #endif // KEEPASSX_TESTENTRY_H