From 9a2f6aee3c17421d87551d6ce721d13b2c298aea Mon Sep 17 00:00:00 2001
From: Aetf <aetf@unlimited-code.works>
Date: Fri, 6 May 2022 17:47:34 -0400
Subject: [PATCH] FdoSecrets: skip entries in recycle bin when searching (fix
 #7933)

---
 src/fdosecrets/objects/Collection.cpp | 15 ++++++------
 src/fdosecrets/objects/Collection.h   |  3 +++
 src/fdosecrets/objects/Service.cpp    |  7 +++---
 tests/gui/TestGuiFdoSecrets.cpp       | 33 +++++++++++++++++++++++++++
 tests/gui/TestGuiFdoSecrets.h         |  1 +
 5 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/src/fdosecrets/objects/Collection.cpp b/src/fdosecrets/objects/Collection.cpp
index 1c870beaa9..68ccc9dcc6 100644
--- a/src/fdosecrets/objects/Collection.cpp
+++ b/src/fdosecrets/objects/Collection.cpp
@@ -276,7 +276,12 @@ namespace FdoSecrets
             EntrySearcher(caseSensitive, skipProtected).search(terms, m_exposedGroup, forceSearch);
         items.reserve(foundEntries.size());
         for (const auto& entry : foundEntries) {
-            items << m_entryToItem.value(entry);
+            const auto item = m_entryToItem.value(entry);
+            // it's possible that we don't have a corresponding item for the entry
+            // this can happen when the recycle bin is below the exposed group.
+            if (item) {
+                items << item;
+            }
         }
         return {};
     }
@@ -458,7 +463,7 @@ namespace FdoSecrets
         });
         // Another possibility is the group being moved to recycle bin.
         connect(m_exposedGroup.data(), &Group::modified, this, [this]() {
-            if (inRecycleBin(m_exposedGroup->parentGroup())) {
+            if (inRecycleBin(m_exposedGroup)) {
                 // reset the exposed group to none
                 FdoSecrets::settings()->setExposedGroup(m_backend->database().data(), {});
             }
@@ -677,11 +682,7 @@ namespace FdoSecrets
     bool Collection::inRecycleBin(Group* group) const
     {
         Q_ASSERT(m_backend);
-
-        if (!group) {
-            // the root group's parent is nullptr, we treat it as not in recycle bin.
-            return false;
-        }
+        Q_ASSERT(group);
 
         if (!m_backend->database()->metadata()) {
             return false;
diff --git a/src/fdosecrets/objects/Collection.h b/src/fdosecrets/objects/Collection.h
index 4bd83d1523..06e8467e53 100644
--- a/src/fdosecrets/objects/Collection.h
+++ b/src/fdosecrets/objects/Collection.h
@@ -107,6 +107,9 @@ namespace FdoSecrets
         DatabaseWidget* backend() const;
         QString backendFilePath() const;
         Service* service() const;
+        /**
+         * similar to Group::isRecycled, but we also return true when the group itself is the recycle bin
+         */
         bool inRecycleBin(Group* group) const;
         bool inRecycleBin(Entry* entry) const;
 
diff --git a/src/fdosecrets/objects/Service.cpp b/src/fdosecrets/objects/Service.cpp
index 60e057e7d0..8cc1383629 100644
--- a/src/fdosecrets/objects/Service.cpp
+++ b/src/fdosecrets/objects/Service.cpp
@@ -258,12 +258,13 @@ namespace FdoSecrets
             }
             // item locked state already covers its collection's locked state
             for (const auto& item : asConst(items)) {
-                bool l;
-                ret = item->locked(client, l);
+                Q_ASSERT(item);
+                bool itemLocked;
+                ret = item->locked(client, itemLocked);
                 if (ret.err()) {
                     return ret;
                 }
-                if (l) {
+                if (itemLocked) {
                     locked.append(item);
                 } else {
                     unlocked.append(item);
diff --git a/tests/gui/TestGuiFdoSecrets.cpp b/tests/gui/TestGuiFdoSecrets.cpp
index a2b5647df6..9bc3e61217 100644
--- a/tests/gui/TestGuiFdoSecrets.cpp
+++ b/tests/gui/TestGuiFdoSecrets.cpp
@@ -1564,6 +1564,39 @@ void TestGuiFdoSecrets::testModifyingExposedGroup()
     }
 }
 
+void TestGuiFdoSecrets::testNoExposeRecycleBin()
+{
+    // when the recycle bin is underneath the exposed group
+    // be careful not to expose entries in there
+
+    FdoSecrets::settings()->setExposedGroup(m_db, m_db->rootGroup()->uuid());
+    m_db->metadata()->setRecycleBinEnabled(true);
+
+    auto entry = m_db->rootGroup()->entries().first();
+    VERIFY(entry);
+    m_db->recycleEntry(entry);
+    processEvents();
+
+    auto service = enableService();
+    VERIFY(service);
+
+    auto coll = getDefaultCollection(service);
+    VERIFY(coll);
+
+    // exposing subgroup does not expose entries in other groups
+    DBUS_GET(itemPaths, coll->items());
+    QSet<Entry*> exposedEntries;
+    for (const auto& itemPath : itemPaths) {
+        exposedEntries << m_plugin->dbus()->pathToObject<Item>(itemPath)->backend();
+    }
+    VERIFY(!exposedEntries.contains(entry));
+
+    // searching should not return the entry
+    DBUS_GET2(unlocked, locked, service->SearchItems({{"Title", entry->title()}}));
+    COMPARE(locked, {});
+    COMPARE(unlocked, {});
+}
+
 void TestGuiFdoSecrets::lockDatabaseInBackend()
 {
     m_dbWidget->lock();
diff --git a/tests/gui/TestGuiFdoSecrets.h b/tests/gui/TestGuiFdoSecrets.h
index 03f84adce6..ab0909b30d 100644
--- a/tests/gui/TestGuiFdoSecrets.h
+++ b/tests/gui/TestGuiFdoSecrets.h
@@ -97,6 +97,7 @@ private slots:
 
     void testExposeSubgroup();
     void testModifyingExposedGroup();
+    void testNoExposeRecycleBin();
 
     void testHiddenFilename();
     void testDuplicateName();