Add an option to EntrySearcher to skip protected attributes

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 (m_skipProtected && entry->attributes()->isProtected(term.word)) {
-            if (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
+        // negate the result if exclude:
-        if ((!found && !term.exclude) || (found && term.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)

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<Entry*> search(const QList<SearchTerm>& searchTerms, const Group* baseGroup, bool forceSearch = false);
     QList<Entry*> search(const QString& searchString, const Group* baseGroup, bool forceSearch = false);
@@ -62,13 +62,14 @@ public:
     QList<Entry*> repeatEntries(const QList<Entry*>& 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<SearchTerm> m_searchTerms;
 

src/fdosecrets/objects/Collection.cpp

@@ -247,19 +247,11 @@ namespace FdoSecrets
 
         QList<EntrySearcher::SearchTerm> 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<Item*>{};
-        }
-
         QList<Item*> 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);

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<Entry> e1(new Entry());
+    e1->setGroup(m_rootGroup);
+
+    e1->attributes()->set("testAttribute", "testE1");
+    e1->attributes()->set("testProtected", "apple", true);
+
+    QScopedPointer<Entry> e2(new Entry());
+    e2->setGroup(m_rootGroup);
+    e2->attributes()->set("testAttribute", "testE2");
+    e2->attributes()->set("testProtected", "banana", true);
+
+    const QList<Entry*> expectE1{e1.data()};
+    const QList<Entry*> expectE2{e2.data()};
+    const QList<Entry*> 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, {});
+}

tests/TestEntrySearcher.h

@@ -37,6 +37,7 @@ private slots:
     void testSearchTermParser();
     void testCustomAttributesAreSearched();
     void testGroup();
+    void testSkipProtected();
 
 private:
     Group* m_rootGroup;