From cd9ef58e982e71103af42613be5354b56ef1136d Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Tue, 9 Nov 2021 23:52:54 +0100 Subject: [PATCH] Implement KDBX 4.1 PreviousParentGroup flag --- src/core/Entry.cpp | 33 +++++++++++++++++++++++-- src/core/Entry.h | 7 +++++- src/core/Group.cpp | 48 +++++++++++++++++++++++++++++++++--- src/core/Group.h | 14 ++++++++--- src/format/KdbxXmlReader.cpp | 12 +++++++-- src/format/KdbxXmlWriter.cpp | 13 ++++++++-- tests/TestEntry.cpp | 46 +++++++++++++++++++++++++++++++++- tests/TestEntry.h | 3 ++- tests/TestGroup.cpp | 39 ++++++++++++++++++++++++++++- tests/TestGroup.h | 3 ++- 10 files changed, 200 insertions(+), 18 deletions(-) diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index e17b3210a..034a295d5 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -1169,7 +1169,7 @@ const Group* Entry::group() const return m_group; } -void Entry::setGroup(Group* group) +void Entry::setGroup(Group* group, bool trackPrevious) { Q_ASSERT(group); @@ -1180,6 +1180,7 @@ void Entry::setGroup(Group* group) if (m_group) { m_group->removeEntry(this); if (m_group->database() && m_group->database() != group->database()) { + setPreviousParentGroup(nullptr); m_group->database()->addDeletedObject(m_uuid); // copy custom icon to the new database @@ -1188,6 +1189,8 @@ void Entry::setGroup(Group* group) group->database()->metadata()->addCustomIcon(iconUuid(), m_group->database()->metadata()->customIcon(iconUuid())); } + } else if (trackPrevious && m_group->database() && group != m_group) { + setPreviousParentGroup(m_group); } } @@ -1375,7 +1378,30 @@ QString Entry::resolveUrl(const QString& url) const } // No valid http URL's found - return QString(""); + return {}; +} + +const Group* Entry::previousParentGroup() const +{ + if (!database() || !database()->rootGroup()) { + return nullptr; + } + return database()->rootGroup()->findGroupByUuid(m_data.previousParentGroupUuid); +} + +QUuid Entry::previousParentGroupUuid() const +{ + return m_data.previousParentGroupUuid; +} + +void Entry::setPreviousParentGroupUuid(const QUuid& uuid) +{ + set(m_data.previousParentGroupUuid, uuid); +} + +void Entry::setPreviousParentGroup(const Group* group) +{ + setPreviousParentGroupUuid(group ? group->uuid() : QUuid()); } bool EntryData::operator==(const EntryData& other) const @@ -1438,6 +1464,9 @@ bool EntryData::equals(const EntryData& other, CompareItemOptions options) const if (::compare(excludeFromReports, other.excludeFromReports, options) != 0) { return false; } + if (::compare(previousParentGroupUuid, other.previousParentGroupUuid, options) != 0) { + return false; + } return true; } diff --git a/src/core/Entry.h b/src/core/Entry.h index 8da09e536..6a19078a1 100644 --- a/src/core/Entry.h +++ b/src/core/Entry.h @@ -66,6 +66,7 @@ struct EntryData QSharedPointer totpSettings; QSharedPointer passwordHealth; bool excludeFromReports; + QUuid previousParentGroupUuid; bool operator==(const EntryData& other) const; bool operator!=(const EntryData& other) const; @@ -106,6 +107,8 @@ public: QString totp() const; QString totpSettingsString() const; QSharedPointer totpSettings() const; + const Group* previousParentGroup() const; + QUuid previousParentGroupUuid() const; int size() const; QString path() const; const QSharedPointer& passwordHealth(); @@ -147,6 +150,8 @@ public: void setExpires(const bool& value); void setExpiryTime(const QDateTime& dateTime); void setTotp(QSharedPointer settings); + void setPreviousParentGroup(const Group* group); + void setPreviousParentGroupUuid(const QUuid& uuid); QList historyItems(); const QList& historyItems() const; @@ -243,7 +248,7 @@ public: Group* group(); const Group* group() const; - void setGroup(Group* group); + void setGroup(Group* group, bool trackPrevious = true); const Database* database() const; Database* database(); diff --git a/src/core/Group.cpp b/src/core/Group.cpp index 6684d1ab9..359b925ab 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -403,7 +403,7 @@ const Group* Group::parentGroup() const return m_parent; } -void Group::setParent(Group* parent, int index) +void Group::setParent(Group* parent, int index, bool trackPrevious) { Q_ASSERT(parent); Q_ASSERT(index >= -1 && index <= parent->children().size()); @@ -428,6 +428,7 @@ void Group::setParent(Group* parent, int index) cleanupParent(); m_parent = parent; if (m_db) { + setPreviousParentGroup(nullptr); recCreateDelObjects(); // copy custom icon to the new database @@ -445,6 +446,9 @@ void Group::setParent(Group* parent, int index) parent->m_children.insert(index, this); } else { emit aboutToMove(this, parent, index); + if (trackPrevious && m_parent != parent) { + setPreviousParentGroup(m_parent); + } m_parent->m_children.removeAll(this); m_parent = parent; QObject::setParent(parent); @@ -585,7 +589,7 @@ Entry* Group::findEntryByUuid(const QUuid& uuid, bool recursive) const return nullptr; } -Entry* Group::findEntryByPath(const QString& entryPath) +Entry* Group::findEntryByPath(const QString& entryPath) const { if (entryPath.isEmpty()) { return nullptr; @@ -647,7 +651,7 @@ Entry* Group::findEntryBySearchTerm(const QString& term, EntryReferenceType refe return nullptr; } -Entry* Group::findEntryByPathRecursive(const QString& entryPath, const QString& basePath) +Entry* Group::findEntryByPathRecursive(const QString& entryPath, const QString& basePath) const { // Return the first entry that matches the full path OR if there is no leading // slash, return the first entry title that matches @@ -843,6 +847,21 @@ Group* Group::findGroupByUuid(const QUuid& uuid) return nullptr; } +const Group* Group::findGroupByUuid(const QUuid& uuid) const +{ + if (uuid.isNull()) { + return nullptr; + } + + for (const Group* group : groupsRecursive(true)) { + if (group->uuid() == uuid) { + return group; + } + } + + return nullptr; +} + Group* Group::findChildByName(const QString& name) { for (Group* group : asConst(m_children)) { @@ -1168,6 +1187,29 @@ void Group::sortChildrenRecursively(bool reverse) emitModified(); } +const Group* Group::previousParentGroup() const +{ + if (!database() || !database()->rootGroup()) { + return nullptr; + } + return database()->rootGroup()->findGroupByUuid(m_data.previousParentGroupUuid); +} + +QUuid Group::previousParentGroupUuid() const +{ + return m_data.previousParentGroupUuid; +} + +void Group::setPreviousParentGroupUuid(const QUuid& uuid) +{ + set(m_data.previousParentGroupUuid, uuid); +} + +void Group::setPreviousParentGroup(const Group* group) +{ + setPreviousParentGroupUuid(group ? group->uuid() : QUuid()); +} + bool Group::GroupData::operator==(const Group::GroupData& other) const { return equals(other, CompareItemDefault); diff --git a/src/core/Group.h b/src/core/Group.h index c283a89a2..5c7321b41 100644 --- a/src/core/Group.h +++ b/src/core/Group.h @@ -69,6 +69,7 @@ public: Group::TriState autoTypeEnabled; Group::TriState searchingEnabled; Group::MergeMode mergeMode; + QUuid previousParentGroupUuid; bool operator==(const GroupData& other) const; bool operator!=(const GroupData& other) const; @@ -101,6 +102,8 @@ public: const CustomData* customData() const; Group::TriState resolveCustomDataTriState(const QString& key, bool checkParent = true) const; void setCustomDataTriState(const QString& key, const Group::TriState& value); + const Group* previousParentGroup() const; + QUuid previousParentGroupUuid() const; bool equals(const Group* other, CompareItemOptions options) const; @@ -110,9 +113,10 @@ public: Group* findChildByName(const QString& name); Entry* findEntryByUuid(const QUuid& uuid, bool recursive = true) const; - Entry* findEntryByPath(const QString& entryPath); + Entry* findEntryByPath(const QString& entryPath) const; Entry* findEntryBySearchTerm(const QString& term, EntryReferenceType referenceType); Group* findGroupByUuid(const QUuid& uuid); + const Group* findGroupByUuid(const QUuid& uuid) const; Group* findGroupByPath(const QString& groupPath); Entry* addEntryWithPath(const QString& entryPath); void setUuid(const QUuid& uuid); @@ -129,13 +133,15 @@ public: void setExpires(bool value); void setExpiryTime(const QDateTime& dateTime); void setMergeMode(MergeMode newMode); + void setPreviousParentGroup(const Group* group); + void setPreviousParentGroupUuid(const QUuid& uuid); bool canUpdateTimeinfo() const; void setUpdateTimeinfo(bool value); Group* parentGroup(); const Group* parentGroup() const; - void setParent(Group* parent, int index = -1); + void setParent(Group* parent, int index = -1, bool trackPrevious = true); QStringList hierarchy(int height = -1) const; bool hasChildren() const; @@ -203,7 +209,7 @@ private: void cleanupParent(); void recCreateDelObjects(); - Entry* findEntryByPathRecursive(const QString& entryPath, const QString& basePath); + Entry* findEntryByPathRecursive(const QString& entryPath, const QString& basePath) const; Group* findGroupByPathRecursive(const QString& groupPath, const QString& basePath); QPointer m_db; @@ -221,7 +227,7 @@ private: friend void Database::setRootGroup(Group* group); friend Entry::~Entry(); - friend void Entry::setGroup(Group* group); + friend void Entry::setGroup(Group* group, bool trackPrevious); }; Q_DECLARE_OPERATORS_FOR_FLAGS(Group::CloneFlags) diff --git a/src/format/KdbxXmlReader.cpp b/src/format/KdbxXmlReader.cpp index b998668e6..504a8db81 100644 --- a/src/format/KdbxXmlReader.cpp +++ b/src/format/KdbxXmlReader.cpp @@ -583,6 +583,10 @@ Group* KdbxXmlReader::parseGroup() parseCustomData(group->customData()); continue; } + if (m_xml.name() == "PreviousParentGroup") { + group->setPreviousParentGroupUuid(readUuid()); + continue; + } skipCurrentElement(); } @@ -602,11 +606,11 @@ Group* KdbxXmlReader::parseGroup() } for (Group* child : asConst(children)) { - child->setParent(group); + child->setParent(group, -1, false); } for (Entry* entry : asConst(entries)) { - entry->setGroup(group); + entry->setGroup(group, false); } return group; @@ -760,6 +764,10 @@ Entry* KdbxXmlReader::parseEntry(bool history) } continue; } + if (m_xml.name() == "PreviousParentGroup") { + entry->setPreviousParentGroupUuid(readUuid()); + continue; + } skipCurrentElement(); } diff --git a/src/format/KdbxXmlWriter.cpp b/src/format/KdbxXmlWriter.cpp index a404fd3c1..5ddfe40d8 100644 --- a/src/format/KdbxXmlWriter.cpp +++ b/src/format/KdbxXmlWriter.cpp @@ -276,6 +276,9 @@ void KdbxXmlWriter::writeGroup(const Group* group) if (m_kdbxVersion >= KeePass2::FILE_VERSION_4) { writeCustomData(group->customData()); + if (!group->previousParentGroupUuid().isNull()) { + writeUuid("PreviousParentGroup", group->previousParentGroupUuid()); + } } const QList& entryList = group->entries(); @@ -344,8 +347,14 @@ void KdbxXmlWriter::writeEntry(const Entry* entry) writeString("OverrideURL", entry->overrideUrl()); writeString("Tags", entry->tags()); writeTimes(entry->timeInfo()); - if (entry->excludeFromReports()) { - writeBool("QualityCheck", false); + + if (m_kdbxVersion >= KeePass2::FILE_VERSION_4) { + if (entry->excludeFromReports()) { + writeBool("QualityCheck", false); + } + if (!entry->previousParentGroupUuid().isNull()) { + writeUuid("PreviousParentGroup", entry->previousParentGroupUuid()); + } } const QList attributesKeyList = entry->attributes()->keys(); diff --git a/tests/TestEntry.cpp b/tests/TestEntry.cpp index a74a8bd8c..3983db101 100644 --- a/tests/TestEntry.cpp +++ b/tests/TestEntry.cpp @@ -614,7 +614,7 @@ void TestEntry::testIsRecycled() QVERIFY(entry1->isRecycled()); } -void TestEntry::testMove() +void TestEntry::testMoveUpDown() { Database db; Group* root = db.rootGroup(); @@ -724,3 +724,47 @@ void TestEntry::testMove() QCOMPARE(root->entries().at(2), entry1); QCOMPARE(root->entries().at(3), entry0); } + +void TestEntry::testPreviousParentGroup() +{ + Database db; + auto* root = db.rootGroup(); + root->setUuid(QUuid::createUuid()); + QVERIFY(!root->uuid().isNull()); + + auto* group1 = new Group(); + group1->setUuid(QUuid::createUuid()); + group1->setParent(root); + QVERIFY(!group1->uuid().isNull()); + + auto* group2 = new Group(); + group2->setParent(root); + group2->setUuid(QUuid::createUuid()); + QVERIFY(!group2->uuid().isNull()); + + auto* entry = new Entry(); + QVERIFY(entry); + QVERIFY(entry->previousParentGroupUuid().isNull()); + QVERIFY(!entry->previousParentGroup()); + + entry->setGroup(root); + QVERIFY(entry->previousParentGroupUuid().isNull()); + QVERIFY(!entry->previousParentGroup()); + + // Previous parent shouldn't be recorded if new and old parent are the same + entry->setGroup(root); + QVERIFY(entry->previousParentGroupUuid().isNull()); + QVERIFY(!entry->previousParentGroup()); + + entry->setGroup(group1); + QVERIFY(entry->previousParentGroupUuid() == root->uuid()); + QVERIFY(entry->previousParentGroup() == root); + + entry->setGroup(group2); + QVERIFY(entry->previousParentGroupUuid() == group1->uuid()); + QVERIFY(entry->previousParentGroup() == group1); + + entry->setGroup(group2); + QVERIFY(entry->previousParentGroupUuid() == group1->uuid()); + QVERIFY(entry->previousParentGroup() == group1); +} diff --git a/tests/TestEntry.h b/tests/TestEntry.h index 9fc7e158b..3bfd8f52d 100644 --- a/tests/TestEntry.h +++ b/tests/TestEntry.h @@ -38,7 +38,8 @@ private slots: void testResolveNonIdPlaceholdersToUuid(); void testResolveClonedEntry(); void testIsRecycled(); - void testMove(); + void testMoveUpDown(); + void testPreviousParentGroup(); }; #endif // KEEPASSX_TESTENTRY_H diff --git a/tests/TestGroup.cpp b/tests/TestGroup.cpp index 55c54d821..8296140d5 100644 --- a/tests/TestGroup.cpp +++ b/tests/TestGroup.cpp @@ -1143,7 +1143,7 @@ void TestGroup::testUsernamesRecursive() QVERIFY(usernames.indexOf("Name2") < usernames.indexOf("Name1")); } -void TestGroup::testMove() +void TestGroup::testMoveUpDown() { Database database; Group* root = database.rootGroup(); @@ -1253,3 +1253,40 @@ void TestGroup::testMove() QCOMPARE(root->entries().at(2), entry1); QCOMPARE(root->entries().at(3), entry0); } + +void TestGroup::testPreviousParentGroup() +{ + Database db; + auto* root = db.rootGroup(); + root->setUuid(QUuid::createUuid()); + QVERIFY(!root->uuid().isNull()); + QVERIFY(!root->previousParentGroup()); + QVERIFY(root->previousParentGroupUuid().isNull()); + + auto* group1 = new Group(); + group1->setUuid(QUuid::createUuid()); + group1->setParent(root); + QVERIFY(!group1->uuid().isNull()); + QVERIFY(!group1->previousParentGroup()); + QVERIFY(group1->previousParentGroupUuid().isNull()); + + auto* group2 = new Group(); + group2->setParent(root); + group2->setUuid(QUuid::createUuid()); + QVERIFY(!group2->uuid().isNull()); + QVERIFY(!group2->previousParentGroup()); + QVERIFY(group2->previousParentGroupUuid().isNull()); + + group1->setParent(group2); + QVERIFY(group1->previousParentGroupUuid() == root->uuid()); + QVERIFY(group1->previousParentGroup() == root); + + // Previous parent shouldn't be recorded if new and old parent are the same + group1->setParent(group2); + QVERIFY(group1->previousParentGroupUuid() == root->uuid()); + QVERIFY(group1->previousParentGroup() == root); + + group1->setParent(root); + QVERIFY(group1->previousParentGroupUuid() == group2->uuid()); + QVERIFY(group1->previousParentGroup() == group2); +} diff --git a/tests/TestGroup.h b/tests/TestGroup.h index b773ea017..0c8fee937 100644 --- a/tests/TestGroup.h +++ b/tests/TestGroup.h @@ -47,7 +47,8 @@ private slots: void testHierarchy(); void testApplyGroupIconRecursively(); void testUsernamesRecursive(); - void testMove(); + void testMoveUpDown(); + void testPreviousParentGroup(); }; #endif // KEEPASSX_TESTGROUP_H