diff --git a/src/core/EntrySearcher.cpp b/src/core/EntrySearcher.cpp index eccffe0a5..3b9472c6f 100644 --- a/src/core/EntrySearcher.cpp +++ b/src/core/EntrySearcher.cpp @@ -21,8 +21,9 @@ #include "core/Group.h" #include "core/Tools.h" -EntrySearcher::EntrySearcher(bool caseSensitive) +EntrySearcher::EntrySearcher(bool caseSensitive, bool skipProtected) : m_caseSensitive(caseSensitive) + , m_skipProtected(skipProtected) , m_termParser(R"re(([-!*+]+)?(?:(\w*):)?(?:(?=")"((?:[^"\\]|\\.)*)"|([^ ]*))( |$))re") // Group 1 = modifiers, Group 2 = field, Group 3 = quoted string, Group 4 = unquoted string { @@ -136,7 +137,7 @@ void EntrySearcher::setCaseSensitive(bool state) m_caseSensitive = state; } -bool EntrySearcher::isCaseSensitive() +bool EntrySearcher::isCaseSensitive() const { return m_caseSensitive; } @@ -150,7 +151,9 @@ bool EntrySearcher::searchEntryImpl(const Entry* entry) // Build a group hierarchy to allow searching for e.g. /group1/subgroup* auto hierarchy = entry->group()->hierarchy().join('/').prepend("/"); - bool found; + // By default, empty term matches every entry. + // However when skipping protected fields, we will recject everything instead + bool found = !m_skipProtected; for (const auto& term : m_searchTerms) { switch (term.field) { case Field::Title: @@ -160,6 +163,9 @@ bool EntrySearcher::searchEntryImpl(const Entry* entry) found = term.regex.match(entry->resolvePlaceholder(entry->username())).hasMatch(); break; case Field::Password: + if (m_skipProtected) { + continue; + } found = term.regex.match(entry->resolvePlaceholder(entry->password())).hasMatch(); break; case Field::Url: @@ -175,8 +181,7 @@ bool EntrySearcher::searchEntryImpl(const Entry* entry) found = !attachments.filter(term.regex).empty(); break; case Field::AttributeValue: - // skip protected attributes - if (entry->attributes()->isProtected(term.word)) { + if (m_skipProtected && entry->attributes()->isProtected(term.word)) { continue; } found = entry->attributes()->contains(term.word) @@ -198,13 +203,18 @@ bool EntrySearcher::searchEntryImpl(const Entry* entry) || term.regex.match(entry->notes()).hasMatch(); } - // Short circuit if we failed to match or we matched and are excluding this term - if ((!found && !term.exclude) || (found && term.exclude)) { + // negate the result if exclude: + // * if found and not excluding, the entry matches + // * if didn't found but excluding, the entry also matches + found = (found && !term.exclude) || (!found && term.exclude); + + // short circuit if we failed the match + if (!found) { return false; } } - return true; + return found; } void EntrySearcher::parseSearchTerms(const QString& searchString) diff --git a/src/core/EntrySearcher.h b/src/core/EntrySearcher.h index 48d15fb05..d1b17557b 100644 --- a/src/core/EntrySearcher.h +++ b/src/core/EntrySearcher.h @@ -51,7 +51,7 @@ public: bool exclude; }; - explicit EntrySearcher(bool caseSensitive = false); + explicit EntrySearcher(bool caseSensitive = false, bool skipProtected = false); QList search(const QList& searchTerms, const Group* baseGroup, bool forceSearch = false); QList search(const QString& searchString, const Group* baseGroup, bool forceSearch = false); @@ -62,13 +62,14 @@ public: QList repeatEntries(const QList& entries); void setCaseSensitive(bool state); - bool isCaseSensitive(); + bool isCaseSensitive() const; private: bool searchEntryImpl(const Entry* entry); void parseSearchTerms(const QString& searchString); bool m_caseSensitive; + bool m_skipProtected; QRegularExpression m_termParser; QList m_searchTerms; diff --git a/src/fdosecrets/objects/Collection.cpp b/src/fdosecrets/objects/Collection.cpp index 9997f4576..a46595ad1 100644 --- a/src/fdosecrets/objects/Collection.cpp +++ b/src/fdosecrets/objects/Collection.cpp @@ -247,19 +247,11 @@ namespace FdoSecrets QList terms; for (auto it = attributes.constBegin(); it != attributes.constEnd(); ++it) { - if (it.key() == EntryAttributes::PasswordKey) { - continue; - } terms << attributeToTerm(it.key(), it.value()); } - // empty terms causes EntrySearcher returns everything - if (terms.isEmpty()) { - return QList{}; - } - QList items; - const auto foundEntries = EntrySearcher().search(terms, m_exposedGroup); + const auto foundEntries = EntrySearcher(false, true).search(terms, m_exposedGroup); items.reserve(foundEntries.size()); for (const auto& entry : foundEntries) { items << m_entryToItem.value(entry); diff --git a/tests/TestEntrySearcher.cpp b/tests/TestEntrySearcher.cpp index 3a0ac6836..1b8b4f5b8 100644 --- a/tests/TestEntrySearcher.cpp +++ b/tests/TestEntrySearcher.cpp @@ -23,6 +23,7 @@ QTEST_GUILESS_MAIN(TestEntrySearcher) void TestEntrySearcher::init() { m_rootGroup = new Group(); + m_entrySearcher = EntrySearcher(); } void TestEntrySearcher::cleanup() @@ -259,6 +260,7 @@ void TestEntrySearcher::testCustomAttributesAreSearched() QCOMPARE(m_searchResult.count(), 2); // protected attributes are ignored + m_entrySearcher = EntrySearcher(false, true); m_searchResult = m_entrySearcher.search("_testAttribute:test _testProtected:testP2", m_rootGroup); QCOMPARE(m_searchResult.count(), 2); } @@ -319,3 +321,52 @@ void TestEntrySearcher::testGroup() m_searchResult = m_entrySearcher.search("g:/group1 search", m_rootGroup); QCOMPARE(m_searchResult.count(), 1); } + +void TestEntrySearcher::testSkipProtected() +{ + QScopedPointer e1(new Entry()); + e1->setGroup(m_rootGroup); + + e1->attributes()->set("testAttribute", "testE1"); + e1->attributes()->set("testProtected", "apple", true); + + QScopedPointer e2(new Entry()); + e2->setGroup(m_rootGroup); + e2->attributes()->set("testAttribute", "testE2"); + e2->attributes()->set("testProtected", "banana", true); + + const QList expectE1{e1.data()}; + const QList expectE2{e2.data()}; + const QList expectBoth{e1.data(), e2.data()}; + + // when not skipping protected, empty term matches everything + m_searchResult = m_entrySearcher.search("", m_rootGroup); + QCOMPARE(m_searchResult, expectBoth); + + // now test the searcher with skipProtected = true + m_entrySearcher = EntrySearcher(false, true); + + // when skipping protected, empty term matches nothing + m_searchResult = m_entrySearcher.search("", m_rootGroup); + QCOMPARE(m_searchResult, {}); + + // having a protected entry in terms should not affect the results in anyways + m_searchResult = m_entrySearcher.search("_testProtected:apple", m_rootGroup); + QCOMPARE(m_searchResult, {}); + m_searchResult = m_entrySearcher.search("_testProtected:apple _testAttribute:testE2", m_rootGroup); + QCOMPARE(m_searchResult, expectE2); + m_searchResult = m_entrySearcher.search("_testProtected:apple _testAttribute:testE1", m_rootGroup); + QCOMPARE(m_searchResult, expectE1); + m_searchResult = + m_entrySearcher.search("_testProtected:apple _testAttribute:testE1 _testAttribute:testE2", m_rootGroup); + QCOMPARE(m_searchResult, {}); + + // also move the protected term around to execurise the short-circut logic + m_searchResult = m_entrySearcher.search("_testAttribute:testE2 _testProtected:apple", m_rootGroup); + QCOMPARE(m_searchResult, expectE2); + m_searchResult = m_entrySearcher.search("_testAttribute:testE1 _testProtected:apple", m_rootGroup); + QCOMPARE(m_searchResult, expectE1); + m_searchResult = + m_entrySearcher.search("_testAttribute:testE1 _testProtected:apple _testAttribute:testE2", m_rootGroup); + QCOMPARE(m_searchResult, {}); +} diff --git a/tests/TestEntrySearcher.h b/tests/TestEntrySearcher.h index 498a00742..4e3e99a43 100644 --- a/tests/TestEntrySearcher.h +++ b/tests/TestEntrySearcher.h @@ -37,6 +37,7 @@ private slots: void testSearchTermParser(); void testCustomAttributesAreSearched(); void testGroup(); + void testSkipProtected(); private: Group* m_rootGroup;