FdoSecrets: Implement unlock before search

Fixes #6942 and fixes #4443

- Return number of deleted entries
- Fix minor memory leak
- FdoSecrets: make all prompt truly async per spec and update tests
    * the waited signal may already be emitted before calling spy.wait(),
      causing the test to fail. This commit checks the count before waiting.
    * check unlock result after waiting for signal
- FdoSecrets: implement unlockBeforeSearch option
- FdoSecrets: make search always work regardless of entry group searching settings, fixes #6942
- FdoSecrets: cleanup gracefully even if some test failed
- FdoSecrets: make it safe to call prompts concurrently
- FdoSecrets: make sure in unit test we click on the correct dialog

Note on the unit tests: objects are not deleted (due to deleteLater event not handled).
So there may be multiple AccessControlDialog. But only one of
it is visible and is the correctly one to click on.

Before this change, a random one may be clicked on, causing the
completed signal never be sent.

share/translations/keepassxc_en.ts

@@ -7880,11 +7880,27 @@ Please consider generating a new key file.</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <source>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;&lt;span style=&quot; font-family:&apos;-apple-system&apos;,&apos;BlinkMacSystemFont&apos;,&apos;Segoe UI&apos;,&apos;Helvetica&apos;,&apos;Arial&apos;,&apos;sans-serif&apos;,&apos;Apple Color Emoji&apos;,&apos;Segoe UI Emoji&apos;; font-size:14px; color:#24292e; background-color:#ffffff;&quot;&gt;This setting does not override disabling recycle bin prompts&lt;/span&gt;&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</source>
+        <source>Confirm when clients request entry deletion</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <source>Confirm when clients request entry deletion</source>
+        <source>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;&lt;span style=&quot;
+           font-family:&apos;-apple-system&apos;,&apos;BlinkMacSystemFont&apos;,&apos;Segoe UI&apos;,&apos;Helvetica&apos;,&apos;Arial&apos;,&apos;sans-serif&apos;,&apos;Apple Color
+           Emoji&apos;,&apos;Segoe UI Emoji&apos;; font-size:14px; color:#24292e; background-color:#ffffff;&quot;&gt;This setting does
+           not override disabling recycle bin prompts&lt;/span&gt;&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;
+          </source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;This improves compatibility with certain applications
+           which search for password without unlocking the database first.&lt;/p&gt;&lt;p&gt;But enabling this may also
+           crash the client if the database can not be unlocked within a certain timeout. (Usually 25s, but may be a
+           different value set in applications.)&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;
+          </source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Prompt to unlock database before searching</source>
         <translation type="unfinished"></translation>
     </message>
 </context>

src/core/Config.cpp

@@ -178,6 +178,7 @@ static const QHash<Config::ConfigKey, ConfigDirective> configStrings = {
     {Config::FdoSecrets_ShowNotification, {QS("FdoSecrets/ShowNotification"), Roaming, true}},
     {Config::FdoSecrets_ConfirmDeleteItem, {QS("FdoSecrets/ConfirmDeleteItem"), Roaming, true}},
     {Config::FdoSecrets_ConfirmAccessItem, {QS("FdoSecrets/ConfirmAccessItem"), Roaming, true}},
+    {Config::FdoSecrets_UnlockBeforeSearch, {QS("FdoSecrets/UnlockBeforeSearch"), Roaming, true}},
 
     // KeeShare
     {Config::KeeShare_QuietSuccess, {QS("KeeShare/QuietSuccess"), Roaming, false}},

src/core/Config.h

@@ -155,6 +155,7 @@ public:
         FdoSecrets_ShowNotification,
         FdoSecrets_ConfirmDeleteItem,
         FdoSecrets_ConfirmAccessItem,
+        FdoSecrets_UnlockBeforeSearch,
 
         KeeShare_QuietSuccess,
         KeeShare_Own,

src/fdosecrets/FdoSecretsSettings.cpp

@@ -83,6 +83,16 @@ namespace FdoSecrets
         config()->set(Config::FdoSecrets_ConfirmAccessItem, confirmAccessItem);
     }
 
+    bool FdoSecretsSettings::unlockBeforeSearch() const
+    {
+        return config()->get(Config::FdoSecrets_UnlockBeforeSearch).toBool();
+    }
+
+    void FdoSecretsSettings::setUnlockBeforeSearch(bool unlockBeforeSearch)
+    {
+        config()->set(Config::FdoSecrets_UnlockBeforeSearch, unlockBeforeSearch);
+    }
+
     QUuid FdoSecretsSettings::exposedGroup(const QSharedPointer<Database>& db) const
     {
         return exposedGroup(db.data());

src/fdosecrets/FdoSecretsSettings.h

@@ -44,6 +44,9 @@ namespace FdoSecrets
         bool confirmAccessItem() const;
         void setConfirmAccessItem(bool confirmAccessItem);
 
+        bool unlockBeforeSearch() const;
+        void setUnlockBeforeSearch(bool unlockBeforeSearch);
+
         // Per db settings
 
         QUuid exposedGroup(const QSharedPointer<Database>& db) const;

src/fdosecrets/dbus/DBusMgr.cpp

@@ -177,14 +177,12 @@ namespace FdoSecrets
         // It's still weak and if the application does a prctl(PR_SET_DUMPABLE, 0) this link cannot be accessed.
         QFileInfo exe(QStringLiteral("/proc/%1/exe").arg(pid));
         info.exePath = exe.canonicalFilePath();
-        qDebug() << "process" << pid << info.exePath;
 
         // /proc/pid/cmdline gives full command line
         QFile cmdline(QStringLiteral("/proc/%1/cmdline").arg(pid));
         if (cmdline.open(QFile::ReadOnly)) {
             info.command = QString::fromLocal8Bit(cmdline.readAll().replace('\0', ' ')).trimmed();
         }
-        qDebug() << "process" << pid << info.command;
 
         // /proc/pid/stat gives ppid, name
         QFile stat(QStringLiteral("/proc/%1/stat").arg(pid));

src/fdosecrets/dbus/DBusObject.h

@@ -85,7 +85,7 @@ namespace FdoSecrets
 
         // Implicitly convert from QDBusError
         DBusResult(QDBusError::ErrorType error) // NOLINT(google-explicit-constructor)
-            : QString(QDBusError::errorString(error))
+            : QString(error == QDBusError::ErrorType::NoError ? "" : QDBusError::errorString(error))
         {
         }
 

src/fdosecrets/objects/Collection.cpp

@@ -24,7 +24,9 @@
 
 #include "core/Tools.h"
 #include "gui/DatabaseWidget.h"
+#include "gui/GuiTools.h"
 
+#include <QEventLoop>
 #include <QFileInfo>
 
 namespace FdoSecrets
@@ -62,9 +64,9 @@ namespace FdoSecrets
 
         // delete all items
         // this has to be done because the backend is actually still there, just we don't expose them
-        // NOTE: Do NOT use a for loop, because Item::doDelete will remove itself from m_items.
+        // NOTE: Do NOT use a for loop, because Item::removeFromDBus will remove itself from m_items.
         while (!m_items.isEmpty()) {
-            m_items.first()->doDelete();
+            m_items.first()->removeFromDBus();
         }
         cleanupConnections();
         dbus()->unregisterObject(this);
@@ -91,7 +93,7 @@ namespace FdoSecrets
     void Collection::reloadBackendOrDelete()
     {
         if (!reloadBackend()) {
-            doDelete();
+            removeFromDBus();
         }
     }
 
@@ -194,31 +196,22 @@ namespace FdoSecrets
         return {};
     }
 
-    DBusResult Collection::remove(const DBusClientPtr& client, PromptBase*& prompt)
+    DBusResult Collection::remove(PromptBase*& prompt)
     {
         auto ret = ensureBackend();
         if (ret.err()) {
             return ret;
         }
 
-        // Delete means close database
         prompt = PromptBase::Create<DeleteCollectionPrompt>(service(), this);
         if (!prompt) {
             return QDBusError::InternalError;
         }
-        if (backendLocked()) {
-            // this won't raise a dialog, immediate execute
-            ret = prompt->prompt(client, {});
-            if (ret.err()) {
-                return ret;
-            }
-            prompt = nullptr;
-        }
-        // defer the close to the prompt
         return {};
     }
 
-    DBusResult Collection::searchItems(const StringStringMap& attributes, QList<Item*>& items)
+    DBusResult
+    Collection::searchItems(const DBusClientPtr& client, const StringStringMap& attributes, QList<Item*>& items)
     {
         items.clear();
 
@@ -226,8 +219,26 @@ namespace FdoSecrets
         if (ret.err()) {
             return ret;
         }
-        ret = ensureUnlocked();
+
-        if (ret.err()) {
+        if (backendLocked() && settings()->unlockBeforeSearch()) {
+            // do a blocking unlock prompt first.
+            // while technically not correct, this should improve compatibility.
+            // see issue #4443
+            auto prompt = PromptBase::Create<UnlockPrompt>(service(), QSet<Collection*>{this}, QSet<Item*>{});
+            if (!prompt) {
+                return QDBusError::InternalError;
+            }
+            // we don't know the windowId to show the prompt correctly,
+            // but the default of showing over the KPXC main window should be good enough
+            ret = prompt->prompt(client, "");
+            // blocking wait
+            QEventLoop loop;
+            connect(prompt, &PromptBase::completed, &loop, &QEventLoop::quit);
+            loop.exec();
+        }
+
+        // check again because the prompt may be cancelled
+        if (backendLocked()) {
             // searchItems should work, whether `this` is locked or not.
             // however, we can't search items the same way as in gnome-keying,
             // because there's no database at all when locked.
@@ -258,7 +269,11 @@ namespace FdoSecrets
             terms << attributeToTerm(it.key(), it.value());
         }
 
-        const auto foundEntries = EntrySearcher(false, true).search(terms, m_exposedGroup);
+        constexpr auto caseSensitive = false;
+        constexpr auto skipProtected = true;
+        constexpr auto forceSearch = true;
+        const auto foundEntries =
+            EntrySearcher(caseSensitive, skipProtected).search(terms, m_exposedGroup, forceSearch);
         items.reserve(foundEntries.size());
         for (const auto& entry : foundEntries) {
             items << m_entryToItem.value(entry);
@@ -298,36 +313,10 @@ namespace FdoSecrets
         if (ret.err()) {
             return ret;
         }
-        ret = ensureUnlocked();
-        if (ret.err()) {
-            return ret;
-        }
 
         item = nullptr;
-        QString itemPath;
 
-        auto iterAttr = properties.find(DBUS_INTERFACE_SECRET_ITEM + ".Attributes");
+        prompt = PromptBase::Create<CreateItemPrompt>(service(), this, properties, secret, replace);
-        if (iterAttr != properties.end()) {
-            // the actual value in iterAttr.value() is QDBusArgument, which represents a structure
-            // and qt has no idea what this corresponds to.
-            // we thus force a conversion to StringStringMap here. The conversion is registered in
-            // DBusTypes.cpp
-            auto attributes = iterAttr.value().value<StringStringMap>();
-
-            itemPath = attributes.value(ItemAttributes::PathKey);
-
-            // check existing item using attributes
-            QList<Item*> existing;
-            ret = searchItems(attributes, existing);
-            if (ret.err()) {
-                return ret;
-            }
-            if (!existing.isEmpty() && replace) {
-                item = existing.front();
-            }
-        }
-
-        prompt = PromptBase::Create<CreateItemPrompt>(service(), this, properties, secret, itemPath, item);
         if (!prompt) {
             return QDBusError::InternalError;
         }
@@ -418,7 +407,7 @@ namespace FdoSecrets
     void Collection::onDatabaseLockChanged()
     {
         if (!reloadBackend()) {
-            doDelete();
+            removeFromDBus();
             return;
         }
         emit collectionLockChanged(backendLocked());
@@ -436,7 +425,7 @@ namespace FdoSecrets
         auto newGroup = m_backend->database()->rootGroup()->findGroupByUuid(newUuid);
         if (!newGroup || inRecycleBin(newGroup)) {
             // no exposed group, delete self
-            doDelete();
+            removeFromDBus();
             return;
         }
 
@@ -505,7 +494,7 @@ namespace FdoSecrets
         // this has to be done because the backend is actually still there
         // just we don't expose them
         for (const auto& item : asConst(m_items)) {
-            item->doDelete();
+            item->removeFromDBus();
         }
 
         // repopulate
@@ -527,7 +516,7 @@ namespace FdoSecrets
         // forward delete signals
         connect(entry->group(), &Group::entryAboutToRemove, item, [item](Entry* toBeRemoved) {
             if (item->backend() == toBeRemoved) {
-                item->doDelete();
+                item->removeFromDBus();
             }
         });
 
@@ -568,17 +557,26 @@ namespace FdoSecrets
     {
         Q_ASSERT(m_backend);
 
-        return m_backend->lock();
+        // do not call m_backend->lock() directly
+        // let the service handle locking which handles concurrent calls
+        return service()->doLockDatabase(m_backend);
     }
 
     void Collection::doUnlock()
     {
         Q_ASSERT(m_backend);
 
-        service()->doUnlockDatabaseInDialog(m_backend);
+        return service()->doUnlockDatabaseInDialog(m_backend);
     }
 
-    void Collection::doDelete()
+    bool Collection::doDelete()
+    {
+        Q_ASSERT(m_backend);
+
+        return service()->doCloseDatabase(m_backend);
+    }
+
+    void Collection::removeFromDBus()
     {
         if (!m_backend) {
             // I'm already deleted
@@ -637,9 +635,18 @@ namespace FdoSecrets
         return !m_backend || !m_backend->database()->isInitialized() || m_backend->isLocked();
     }
 
-    void Collection::doDeleteEntries(QList<Entry*> entries)
+    bool Collection::doDeleteEntry(Entry* entry)
     {
-        m_backend->deleteEntries(std::move(entries), FdoSecrets::settings()->confirmDeleteItem());
+        // Confirm entry removal before moving forward
+        bool permanent = inRecycleBin(entry) || !m_backend->database()->metadata()->recycleBinEnabled();
+        if (FdoSecrets::settings()->confirmDeleteItem()
+            && !GuiTools::confirmDeleteEntries(m_backend, {entry}, permanent)) {
+            return false;
+        }
+
+        auto num = GuiTools::deleteEntriesResolveReferences(m_backend, {entry}, permanent);
+
+        return num != 0;
     }
 
     Group* Collection::findCreateGroupByPath(const QString& groupPath)

src/fdosecrets/objects/Collection.h

@@ -63,8 +63,10 @@ namespace FdoSecrets
 
         Q_INVOKABLE DBUS_PROPERTY DBusResult modified(qulonglong& modified) const;
 
-        Q_INVOKABLE DBusResult remove(const DBusClientPtr& client, PromptBase*& prompt);
+        Q_INVOKABLE DBusResult remove(PromptBase*& prompt);
-        Q_INVOKABLE DBusResult searchItems(const StringStringMap& attributes, QList<Item*>& items);
+        Q_INVOKABLE DBusResult searchItems(const DBusClientPtr& client,
+                                           const StringStringMap& attributes,
+                                           QList<Item*>& items);
         Q_INVOKABLE DBusResult
         createItem(const QVariantMap& properties, const Secret& secret, bool replace, Item*& item, PromptBase*& prompt);
 
@@ -112,14 +114,18 @@ namespace FdoSecrets
 
     public slots:
         // expose some methods for Prompt to use
-        bool doLock();
-        void doUnlock();
-        Item* doNewItem(const DBusClientPtr& client, QString itemPath);
-        // will remove self
-        void doDelete();
 
-        // delete the Entry in backend from this collection
+        bool doLock();
-        void doDeleteEntries(QList<Entry*> entries);
+        // actually only closes database tab in KPXC
+        bool doDelete();
+        // Async, finish signal doneUnlockCollection
+        void doUnlock();
+
+        bool doDeleteEntry(Entry* entry);
+        Item* doNewItem(const DBusClientPtr& client, QString itemPath);
+
+        // Only delete from dbus, will remove self. Do not affect database in KPXC
+        void removeFromDBus();
 
         // force reload info from backend, potentially delete self
         bool reloadBackend();

src/fdosecrets/objects/Item.cpp

@@ -335,7 +335,14 @@ namespace FdoSecrets
         return m_backend;
     }
 
-    void Item::doDelete()
+    bool Item::doDelete()
+    {
+        Q_ASSERT(m_backend);
+
+        return collection()->doDeleteEntry(m_backend);
+    }
+
+    void Item::removeFromDBus()
     {
         emit itemAboutToDelete();
 

src/fdosecrets/objects/Item.h

@@ -91,8 +91,13 @@ namespace FdoSecrets
         QString path() const;
 
     public slots:
-        void doDelete();
+        // will actually delete the entry in KPXC
+        bool doDelete();
 
+        // Only delete from dbus, will remove self. Do not affect database in KPXC
+        void removeFromDBus();
+
+    private slots:
         /**
          * Check if the backend is a valid object, send error reply if not.
          * @return No error if the backend is valid.

src/fdosecrets/objects/Prompt.cpp

@@ -32,6 +32,7 @@
 
 namespace FdoSecrets
 {
+    const PromptResult PromptResult::Pending{PromptResult::AsyncPending};
 
     PromptBase::PromptBase(Service* parent)
         : DBusObject(parent)
@@ -60,19 +61,7 @@ namespace FdoSecrets
         return qobject_cast<Service*>(parent());
     }
 
-    DBusResult PromptBase::dismiss()
+    DBusResult PromptBase::prompt(const DBusClientPtr& client, const QString& windowId)
-    {
-        emit completed(true, "");
-        return {};
-    }
-
-    DeleteCollectionPrompt::DeleteCollectionPrompt(Service* parent, Collection* coll)
-        : PromptBase(parent)
-        , m_collection(coll)
-    {
-    }
-
-    DBusResult DeleteCollectionPrompt::prompt(const DBusClientPtr&, const QString& windowId)
     {
         if (thread() != QThread::currentThread()) {
             DBusResult ret;
@@ -81,19 +70,62 @@ namespace FdoSecrets
             return ret;
         }
 
-        if (!m_collection) {
+        QWeakPointer<DBusClient> weak = client;
-            return DBusResult(DBUS_ERROR_SECRET_NO_SUCH_OBJECT);
+        // execute the actual prompt method in event loop to avoid block this method
-        }
+        QTimer::singleShot(0, this, [this, weak, windowId]() {
-
+            auto c = weak.lock();
-        MessageBox::OverrideParent override(findWindow(windowId));
+            if (!c) {
-        // only need to delete in backend, collection will react itself.
+                return;
-        auto accepted = service()->doCloseDatabase(m_collection->backend());
+            }
-
+            if (m_signalSent) {
-        emit completed(!accepted, "");
+                return;
-
+            }
+            auto res = promptSync(c, windowId);
+            if (!res.isPending()) {
+                finishPrompt(res.isDismiss());
+            }
+        });
         return {};
     }
 
+    DBusResult PromptBase::dismiss()
+    {
+        finishPrompt(true);
+        return {};
+    }
+
+    QVariant PromptBase::currentResult() const
+    {
+        return "";
+    }
+
+    void PromptBase::finishPrompt(bool dismissed)
+    {
+        if (m_signalSent) {
+            return;
+        }
+        m_signalSent = true;
+        emit completed(dismissed, currentResult());
+    }
+
+    DeleteCollectionPrompt::DeleteCollectionPrompt(Service* parent, Collection* coll)
+        : PromptBase(parent)
+        , m_collection(coll)
+    {
+    }
+
+    PromptResult DeleteCollectionPrompt::promptSync(const DBusClientPtr&, const QString& windowId)
+    {
+        MessageBox::OverrideParent override(findWindow(windowId));
+
+        // if m_collection is already gone then treat as deletion accepted
+        auto accepted = true;
+        if (m_collection) {
+            accepted = m_collection->doDelete();
+        }
+        return PromptResult::accepted(accepted);
+    }
+
     CreateCollectionPrompt::CreateCollectionPrompt(Service* parent, QVariantMap properties, QString alias)
         : PromptBase(parent)
         , m_properties(std::move(properties))
@@ -101,43 +133,45 @@ namespace FdoSecrets
     {
     }
 
-    DBusResult CreateCollectionPrompt::prompt(const DBusClientPtr&, const QString& windowId)
+    QVariant CreateCollectionPrompt::currentResult() const
     {
-        if (thread() != QThread::currentThread()) {
+        return QVariant::fromValue(DBusMgr::objectPathSafe(m_coll));
-            DBusResult ret;
+    }
-            QMetaObject::invokeMethod(
-                this, "prompt", Qt::BlockingQueuedConnection, Q_ARG(QString, windowId), Q_RETURN_ARG(DBusResult, ret));
-            return ret;
-        }
 
+    PromptResult CreateCollectionPrompt::promptSync(const DBusClientPtr&, const QString& windowId)
+    {
         MessageBox::OverrideParent override(findWindow(windowId));
 
-        auto coll = service()->doNewDatabase();
+        bool created = false;
-        if (!coll) {
+        // collection with the alias may be created since the prompt was created
-            return dismiss();
+        auto ret = service()->readAlias(m_alias, m_coll);
-        }
-
-        auto ret = coll->setProperties(m_properties);
         if (ret.err()) {
-            coll->doDelete();
+            return ret;
-            return dismiss();
+        }
+        if (!m_coll) {
+            created = true;
+            m_coll = service()->doNewDatabase();
+            if (!m_coll) {
+                return PromptResult::accepted(false);
+            }
+        }
+        ret = m_coll->setProperties(m_properties);
+        if (ret.err()) {
+            if (created) {
+                m_coll->removeFromDBus();
+            }
+            return ret;
         }
         if (!m_alias.isEmpty()) {
-            ret = coll->addAlias(m_alias);
+            ret = m_coll->addAlias(m_alias);
             if (ret.err()) {
-                coll->doDelete();
+                if (created) {
-                return dismiss();
+                    m_coll->removeFromDBus();
+                }
+                return ret;
             }
         }
 
-        emit completed(false, QVariant::fromValue(coll->objectPath()));
-
-        return {};
-    }
-
-    DBusResult CreateCollectionPrompt::dismiss()
-    {
-        emit completed(true, QVariant::fromValue(DBusMgr::objectPathSafe(nullptr)));
         return {};
     }
 
@@ -150,35 +184,24 @@ namespace FdoSecrets
         }
     }
 
-    DBusResult LockCollectionsPrompt::prompt(const DBusClientPtr&, const QString& windowId)
+    QVariant LockCollectionsPrompt::currentResult() const
     {
-        if (thread() != QThread::currentThread()) {
+        return QVariant::fromValue(m_locked);
-            DBusResult ret;
+    }
-            QMetaObject::invokeMethod(
-                this, "prompt", Qt::BlockingQueuedConnection, Q_ARG(QString, windowId), Q_RETURN_ARG(DBusResult, ret));
-            return ret;
-        }
 
+    PromptResult LockCollectionsPrompt::promptSync(const DBusClientPtr&, const QString& windowId)
+    {
         MessageBox::OverrideParent override(findWindow(windowId));
 
         for (const auto& c : asConst(m_collections)) {
             if (c) {
-                if (!c->doLock()) {
+                auto accepted = c->doLock();
-                    return dismiss();
+                if (accepted) {
+                    m_locked << c->objectPath();
                 }
-                m_locked << c->objectPath();
             }
         }
-
+        return PromptResult::accepted(m_locked.size() == m_collections.size());
-        emit completed(false, QVariant::fromValue(m_locked));
-
-        return {};
-    }
-
-    DBusResult LockCollectionsPrompt::dismiss()
-    {
-        emit completed(true, QVariant::fromValue(m_locked));
-        return {};
     }
 
     UnlockPrompt::UnlockPrompt(Service* parent, const QSet<Collection*>& colls, const QSet<Item*>& items)
@@ -194,15 +217,13 @@ namespace FdoSecrets
         }
     }
 
-    DBusResult UnlockPrompt::prompt(const DBusClientPtr& client, const QString& windowId)
+    QVariant UnlockPrompt::currentResult() const
     {
-        if (thread() != QThread::currentThread()) {
+        return QVariant::fromValue(m_unlocked);
-            DBusResult ret;
+    }
-            QMetaObject::invokeMethod(
-                this, "prompt", Qt::BlockingQueuedConnection, Q_ARG(QString, windowId), Q_RETURN_ARG(DBusResult, ret));
-            return ret;
-        }
 
+    PromptResult UnlockPrompt::promptSync(const DBusClientPtr& client, const QString& windowId)
+    {
         MessageBox::OverrideParent override(findWindow(windowId));
 
         // for use in unlockItems
@@ -214,6 +235,7 @@ namespace FdoSecrets
         for (const auto& c : asConst(m_collections)) {
             if (c) {
                 // doUnlock is nonblocking, execution will continue in collectionUnlockFinished
+                // it is ok to call doUnlock multiple times before it's actually unlocked by the user
                 c->doUnlock();
                 waitingForCollections = true;
             }
@@ -222,11 +244,10 @@ namespace FdoSecrets
         // unlock items directly if no collection unlocking pending
         // o.w. do it in collectionUnlockFinished
         if (!waitingForCollections) {
-            // do not block the current method
+            unlockItems();
-            QTimer::singleShot(0, this, &UnlockPrompt::unlockItems);
         }
 
-        return {};
+        return PromptResult::Pending;
     }
 
     void UnlockPrompt::collectionUnlockFinished(bool accepted)
@@ -248,8 +269,7 @@ namespace FdoSecrets
             m_unlocked << coll->objectPath();
         } else {
             m_numRejected += 1;
-            // no longer need to unlock the item if its containing collection
+            // no longer need to unlock the item if its containing collection didn't unlock.
-            // didn't unlock.
             m_items.remove(coll);
         }
 
@@ -270,7 +290,7 @@ namespace FdoSecrets
 
         // flatten to list of entries
         QList<Entry*> entries;
-        for (const auto& itemsPerColl : m_items.values()) {
+        for (const auto& itemsPerColl : asConst(m_items)) {
             for (const auto& item : itemsPerColl) {
                 if (!item) {
                     m_numRejected += 1;
@@ -306,6 +326,7 @@ namespace FdoSecrets
         auto client = m_client.lock();
         if (!client) {
             // client already gone
+            qDebug() << "DBus client gone before item unlocking finish";
             return;
         }
         for (auto it = decisions.constBegin(); it != decisions.constEnd(); ++it) {
@@ -326,13 +347,7 @@ namespace FdoSecrets
         }
         // if anything is not unlocked, treat the whole prompt as dismissed
         // so the client has a chance to handle the error
-        emit completed(m_numRejected > 0, QVariant::fromValue(m_unlocked));
+        finishPrompt(m_numRejected > 0);
-    }
-
-    DBusResult UnlockPrompt::dismiss()
-    {
-        emit completed(true, QVariant::fromValue(m_unlocked));
-        return {};
     }
 
     DeleteItemPrompt::DeleteItemPrompt(Service* parent, Item* item)
@@ -341,117 +356,114 @@ namespace FdoSecrets
     {
     }
 
-    DBusResult DeleteItemPrompt::prompt(const DBusClientPtr&, const QString& windowId)
+    PromptResult DeleteItemPrompt::promptSync(const DBusClientPtr&, const QString& windowId)
     {
-        if (thread() != QThread::currentThread()) {
-            DBusResult ret;
-            QMetaObject::invokeMethod(
-                this, "prompt", Qt::BlockingQueuedConnection, Q_ARG(QString, windowId), Q_RETURN_ARG(DBusResult, ret));
-            return ret;
-        }
-
         MessageBox::OverrideParent override(findWindow(windowId));
 
-        // delete item's backend. Item will be notified after the backend is deleted.
+        // if m_item is gone, assume it's already deleted
+        bool deleted = true;
         if (m_item) {
-            m_item->collection()->doDeleteEntries({m_item->backend()});
+            deleted = m_item->doDelete();
         }
-
+        return PromptResult::accepted(deleted);
-        emit completed(false, "");
-
-        return {};
     }
 
     CreateItemPrompt::CreateItemPrompt(Service* parent,
                                        Collection* coll,
                                        QVariantMap properties,
                                        Secret secret,
-                                       QString itemPath,
+                                       bool replace)
-                                       Item* existing)
         : PromptBase(parent)
         , m_coll(coll)
         , m_properties(std::move(properties))
         , m_secret(std::move(secret))
-        , m_itemPath(std::move(itemPath))
+        , m_replace(replace)
-        , m_item(existing)
+        , m_item(nullptr)
         // session aliveness also need to be tracked, for potential use later in updateItem
         , m_sess(m_secret.session)
     {
     }
 
-    DBusResult CreateItemPrompt::prompt(const DBusClientPtr& client, const QString& windowId)
+    QVariant CreateItemPrompt::currentResult() const
     {
-        if (thread() != QThread::currentThread()) {
+        return QVariant::fromValue(DBusMgr::objectPathSafe(m_item));
-            DBusResult ret;
+    }
-            QMetaObject::invokeMethod(
+
-                this, "prompt", Qt::BlockingQueuedConnection, Q_ARG(QString, windowId), Q_RETURN_ARG(DBusResult, ret));
+    PromptResult CreateItemPrompt::promptSync(const DBusClientPtr& client, const QString& windowId)
-            return ret;
+    {
+        if (!m_coll) {
+            return PromptResult::accepted(false);
         }
 
-        MessageBox::OverrideParent override(findWindow(windowId));
+        bool locked = true;
-
+        auto ret = m_coll->locked(locked);
-        if (!m_coll) {
+        if (locked) {
-            return dismiss();
+            // collection was locked
+            return DBusResult{DBUS_ERROR_SECRET_IS_LOCKED};
         }
 
         // save a weak reference to the client which may be used asynchronously later
         m_client = client;
 
-        // the item doesn't exists yet, create it
+        // get itemPath to create item and
+        // try finding an existing item using attributes
+        QString itemPath{};
+        auto iterAttr = m_properties.find(DBUS_INTERFACE_SECRET_ITEM + ".Attributes");
+        if (iterAttr != m_properties.end()) {
+            // the actual value in iterAttr.value() is QDBusArgument, which represents a structure
+            // and qt has no idea what this corresponds to.
+            // we thus force a conversion to StringStringMap here. The conversion is registered in
+            // DBusTypes.cpp
+            auto attributes = iterAttr.value().value<StringStringMap>();
+
+            itemPath = attributes.value(ItemAttributes::PathKey);
+
+            // check existing item using attributes
+            QList<Item*> existing;
+            ret = m_coll->searchItems(client, attributes, existing);
+            if (ret.err()) {
+                return ret;
+            }
+            if (!existing.isEmpty() && m_replace) {
+                m_item = existing.front();
+            }
+        }
+
         if (!m_item) {
-            m_item = m_coll->doNewItem(client, m_itemPath);
+            // the item doesn't exist yet, create it
+            m_item = m_coll->doNewItem(client, itemPath);
             if (!m_item) {
                 // may happen if entry somehow ends up in recycle bin
-                return DBusResult(DBUS_ERROR_SECRET_NO_SUCH_OBJECT);
+                return DBusResult{DBUS_ERROR_SECRET_NO_SUCH_OBJECT};
             }
+        }
 
-            auto ret = updateItem();
+        // the item may be locked due to authorization
-            if (ret.err()) {
+        ret = m_item->locked(client, locked);
-                m_item->doDelete();
+        if (ret.err()) {
-                return ret;
+            return ret;
+        }
+        if (locked) {
+            // give the user a chance to unlock the item
+            auto prompt = PromptBase::Create<UnlockPrompt>(service(), QSet<Collection*>{}, QSet<Item*>{m_item});
+            if (!prompt) {
+                return DBusResult{QDBusError::InternalError};
             }
-            emit completed(false, QVariant::fromValue(m_item->objectPath()));
+            // postpone anything after the confirmation
-        } else {
+            connect(prompt, &PromptBase::completed, this, [this]() {
-            bool locked = false;
+                auto res = updateItem();
-            auto ret = m_item->locked(client, locked);
+                finishPrompt(res.err());
+            });
+
+            ret = prompt->prompt(client, windowId);
             if (ret.err()) {
                 return ret;
             }
-            if (locked) {
+            return PromptResult::Pending;
-                // give the user a chance to unlock the item
-                auto prompt = PromptBase::Create<UnlockPrompt>(service(), QSet<Collection*>{}, QSet<Item*>{m_item});
-                if (!prompt) {
-                    return QDBusError::InternalError;
-                }
-                // postpone anything after the confirmation
-                connect(prompt, &PromptBase::completed, this, &CreateItemPrompt::itemUnlocked);
-                return prompt->prompt(client, windowId);
-            } else {
-                ret = updateItem();
-                if (ret.err()) {
-                    return ret;
-                }
-                emit completed(false, QVariant::fromValue(m_item->objectPath()));
-            }
         }
-        return {};
-    }
 
-    DBusResult CreateItemPrompt::dismiss()
+        // the item can be updated directly
-    {
+        return updateItem();
-        emit completed(true, QVariant::fromValue(DBusMgr::objectPathSafe(nullptr)));
-        return {};
-    }
-
-    void CreateItemPrompt::itemUnlocked(bool dismissed, const QVariant& result)
-    {
-        auto unlocked = result.value<QList<QDBusObjectPath>>();
-        if (!unlocked.isEmpty()) {
-            // in theory we should check if the object path matches m_item, but a mismatch should not happen,
-            // because we control the unlock prompt ourselves
-            updateItem();
-        }
-        emit completed(dismissed, QVariant::fromValue(DBusMgr::objectPathSafe(m_item)));
     }
 
     DBusResult CreateItemPrompt::updateItem()

src/fdosecrets/objects/Prompt.h

@@ -32,14 +32,60 @@ namespace FdoSecrets
 
     class Service;
 
+    // a simple helper class to auto convert
+    // true/false, DBusResult and Pending values
+    class PromptResult
+    {
+        enum Value
+        {
+            Accepted,
+            Dismissed,
+            AsyncPending,
+        };
+        const Value value;
+
+        explicit PromptResult(Value v) noexcept
+            : value(v)
+        {
+        }
+        explicit PromptResult(bool accepted)
+            : value(accepted ? Accepted : Dismissed)
+        {
+        }
+
+    public:
+        PromptResult()
+            : PromptResult(true)
+        {
+        }
+        PromptResult(const DBusResult& res) // NOLINT(google-explicit-constructor)
+            : PromptResult(res.ok())
+        {
+        }
+
+        static const PromptResult Pending;
+        static PromptResult accepted(bool accepted)
+        {
+            return PromptResult{accepted};
+        }
+
+        bool isDismiss() const
+        {
+            return value == Dismissed;
+        }
+        bool isPending() const
+        {
+            return value == AsyncPending;
+        }
+    };
+
     class PromptBase : public DBusObject
     {
         Q_OBJECT
         Q_CLASSINFO("D-Bus Interface", DBUS_INTERFACE_SECRET_PROMPT_LITERAL)
     public:
-        Q_INVOKABLE virtual DBusResult prompt(const DBusClientPtr& client, const QString& windowId) = 0;
+        Q_INVOKABLE DBusResult prompt(const DBusClientPtr& client, const QString& windowId);
-
+        Q_INVOKABLE DBusResult dismiss();
-        Q_INVOKABLE virtual DBusResult dismiss();
 
         template <typename PROMPT, typename... ARGS> static PromptBase* Create(Service* parent, ARGS&&... args)
         {
@@ -57,8 +103,15 @@ namespace FdoSecrets
     protected:
         explicit PromptBase(Service* parent);
 
+        virtual PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) = 0;
+        virtual QVariant currentResult() const;
+
         QWindow* findWindow(const QString& windowId);
         Service* service() const;
+        void finishPrompt(bool dismissed);
+
+    private:
+        bool m_signalSent = false;
     };
 
     class Collection;
@@ -66,14 +119,11 @@ namespace FdoSecrets
     class DeleteCollectionPrompt : public PromptBase
     {
         Q_OBJECT
+        friend class PromptBase;
 
         explicit DeleteCollectionPrompt(Service* parent, Collection* coll);
 
-    public:
+        PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
-        DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
-
-    private:
-        friend class PromptBase;
 
         QPointer<Collection> m_collection;
     };
@@ -81,32 +131,27 @@ namespace FdoSecrets
     class CreateCollectionPrompt : public PromptBase
     {
         Q_OBJECT
+        friend class PromptBase;
 
         explicit CreateCollectionPrompt(Service* parent, QVariantMap properties, QString alias);
 
-    public:
+        PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
-        DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
+        QVariant currentResult() const override;
-        DBusResult dismiss() override;
-
-    private:
-        friend class PromptBase;
 
         QVariantMap m_properties;
         QString m_alias;
+        Collection* m_coll{};
     };
 
     class LockCollectionsPrompt : public PromptBase
     {
         Q_OBJECT
+        friend class PromptBase;
 
         explicit LockCollectionsPrompt(Service* parent, const QList<Collection*>& colls);
 
-    public:
+        PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
-        DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
+        QVariant currentResult() const override;
-        DBusResult dismiss() override;
-
-    private:
-        friend class PromptBase;
 
         QList<QPointer<Collection>> m_collections;
         QList<QDBusObjectPath> m_locked;
@@ -116,22 +161,17 @@ namespace FdoSecrets
     class UnlockPrompt : public PromptBase
     {
         Q_OBJECT
+        friend class PromptBase;
 
         explicit UnlockPrompt(Service* parent, const QSet<Collection*>& colls, const QSet<Item*>& items);
 
-    public:
+        PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
-        DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
+        QVariant currentResult() const override;
-        DBusResult dismiss() override;
 
-    private slots:
         void collectionUnlockFinished(bool accepted);
         void itemUnlockFinished(const QHash<Entry*, AuthDecision>& results);
-
-    private:
         void unlockItems();
 
-        friend class PromptBase;
-
         static constexpr auto FdoSecretsBackend = "FdoSecretsBackend";
 
         QList<QPointer<Collection>> m_collections;
@@ -148,14 +188,11 @@ namespace FdoSecrets
     class DeleteItemPrompt : public PromptBase
     {
         Q_OBJECT
+        friend class PromptBase;
 
         explicit DeleteItemPrompt(Service* parent, Item* item);
 
-    public:
+        PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
-        DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
-
-    private:
-        friend class PromptBase;
 
         QPointer<Item> m_item;
     };
@@ -163,29 +200,24 @@ namespace FdoSecrets
     class CreateItemPrompt : public PromptBase
     {
         Q_OBJECT
+        friend class PromptBase;
 
         explicit CreateItemPrompt(Service* parent,
                                   Collection* coll,
                                   QVariantMap properties,
                                   Secret secret,
-                                  QString itemPath,
+                                  bool replace);
-                                  Item* existing);
 
-    public:
+        PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
-        DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
+        QVariant currentResult() const override;
-        DBusResult dismiss() override;
-    private slots:
-        void itemUnlocked(bool dismissed, const QVariant& result);
 
-    private:
         DBusResult updateItem();
 
-        friend class PromptBase;
-
         QPointer<Collection> m_coll;
         QVariantMap m_properties;
         Secret m_secret;
-        QString m_itemPath;
+        bool m_replace;
+
         QPointer<Item> m_item;
 
         QPointer<const Session> m_sess;

src/fdosecrets/objects/Service.cpp

@@ -52,8 +52,10 @@ namespace FdoSecrets
         , m_databases(std::move(dbTabs))
         , m_insideEnsureDefaultAlias(false)
     {
-        connect(
+        connect(m_databases,
-            m_databases, &DatabaseTabWidget::databaseUnlockDialogFinished, this, &Service::doneUnlockDatabaseInDialog);
+                &DatabaseTabWidget::databaseUnlockDialogFinished,
+                this,
+                &Service::onDatabaseUnlockDialogFinished);
     }
 
     Service::~Service() = default;
@@ -86,7 +88,7 @@ namespace FdoSecrets
     {
         // The Collection will monitor the database's exposed group.
         // When the Collection finds that no exposed group, it will delete itself.
-        // Thus the service also needs to monitor it and recreate the collection if the user changes
+        // Thus, the service also needs to monitor it and recreate the collection if the user changes
         // from no exposed to exposed something.
         connect(dbWidget, &DatabaseWidget::databaseReplaced, this, [this, dbWidget]() {
             monitorDatabaseExposedGroup(dbWidget);
@@ -119,16 +121,16 @@ namespace FdoSecrets
         // m_backend may already be reset to nullptr
         // We want to remove the collection object from dbus as early as possible, to avoid
         // race conditions when deleteLater was called on the m_backend, but not delivered yet,
-        // and new method calls from dbus occurred. Therefore we can't rely on the destroyed
+        // and new method calls from dbus occurred. Therefore, we can't rely on the destroyed
         // signal on m_backend.
         // bind to coll lifespan
         connect(m_databases.data(), &DatabaseTabWidget::databaseClosed, coll, [coll](const QString& filePath) {
             if (filePath == coll->backendFilePath()) {
-                coll->doDelete();
+                coll->removeFromDBus();
             }
         });
 
-        // actual load, must after updates to m_collections, because the reload may trigger
+        // actual load, must after updates to m_collections, because reloading may trigger
         // another onDatabaseTabOpen, and m_collections will be used to prevent recursion.
         if (!coll->reloadBackend()) {
             // error in dbus
@@ -250,7 +252,7 @@ namespace FdoSecrets
 
         for (const auto& coll : asConst(colls)) {
             QList<Item*> items;
-            ret = coll->searchItems(attributes, items);
+            ret = coll->searchItems(client, attributes, items);
             if (ret.err()) {
                 return ret;
             }
@@ -282,7 +284,7 @@ namespace FdoSecrets
         itemsToUnlock.reserve(objects.size());
 
         for (const auto& obj : asConst(objects)) {
-            // the object is either an item or an collection
+            // the object is either an item or a collection
             auto item = qobject_cast<Item*>(obj);
             auto coll = item ? item->collection() : qobject_cast<Collection*>(obj);
             // either way there should be a collection
@@ -317,6 +319,7 @@ namespace FdoSecrets
             }
         }
 
+        prompt = nullptr;
         if (!collectionsToUnlock.isEmpty() || !itemsToUnlock.isEmpty()) {
             prompt = PromptBase::Create<UnlockPrompt>(this, collectionsToUnlock, itemsToUnlock);
             if (!prompt) {
@@ -493,9 +496,44 @@ namespace FdoSecrets
         m_plugin->emitRequestSwitchToDatabases();
     }
 
+    bool Service::doLockDatabase(DatabaseWidget* dbWidget)
+    {
+        // return immediately if the db is already unlocked
+        if (dbWidget && dbWidget->isLocked()) {
+            return true;
+        }
+
+        // mark the db as being unlocked to prevent multiple dialogs for the same db
+        if (m_lockingDb.contains(dbWidget)) {
+            return true;
+        }
+        m_lockingDb.insert(dbWidget);
+        auto ret = dbWidget->lock();
+        m_lockingDb.remove(dbWidget);
+
+        return ret;
+    }
+
     void Service::doUnlockDatabaseInDialog(DatabaseWidget* dbWidget)
     {
+        // return immediately if the db is already unlocked
+        if (dbWidget && !dbWidget->isLocked()) {
+            emit doneUnlockDatabaseInDialog(true, dbWidget);
+            return;
+        }
+
+        // mark the db as being unlocked to prevent multiple dialogs for the same db
+        if (m_unlockingDb.contains(dbWidget)) {
+            return;
+        }
+        m_unlockingDb.insert(dbWidget);
+
         m_databases->unlockDatabaseInDialog(dbWidget, DatabaseOpenDialog::Intent::None);
     }
 
+    void Service::onDatabaseUnlockDialogFinished(bool accepted, DatabaseWidget* dbWidget)
+    {
+        m_unlockingDb.remove(dbWidget);
+        emit doneUnlockDatabaseInDialog(accepted, dbWidget);
+    }
 } // namespace FdoSecrets

src/fdosecrets/objects/Service.h

@@ -96,8 +96,8 @@ namespace FdoSecrets
 
         /**
          * Finish signal for async action doUnlockDatabaseInDialog
-         * @param accepted If false, the action is canceled by the user
+         * @param accepted If false, the action is cancelled by the user
-         * @param dbWidget The unlocked the dbWidget if succeed
+         * @param dbWidget The dbWidget the action is on
          */
         void doneUnlockDatabaseInDialog(bool accepted, DatabaseWidget* dbWidget);
 
@@ -114,6 +114,7 @@ namespace FdoSecrets
         }
 
     public slots:
+        bool doLockDatabase(DatabaseWidget* dbWidget);
         bool doCloseDatabase(DatabaseWidget* dbWidget);
         Collection* doNewDatabase();
         void doSwitchToDatabaseSettings(DatabaseWidget* dbWidget);
@@ -135,6 +136,8 @@ namespace FdoSecrets
 
         void onCollectionAliasRemoved(const QString& alias);
 
+        void onDatabaseUnlockDialogFinished(bool accepted, DatabaseWidget* dbWidget);
+
     private:
         bool initialize();
 
@@ -153,16 +156,18 @@ namespace FdoSecrets
         Collection* findCollection(const DatabaseWidget* db) const;
 
     private:
-        FdoSecretsPlugin* m_plugin;
+        FdoSecretsPlugin* m_plugin{nullptr};
-        QPointer<DatabaseTabWidget> m_databases;
+        QPointer<DatabaseTabWidget> m_databases{};
 
-        QHash<QString, Collection*> m_aliases;
+        QHash<QString, Collection*> m_aliases{};
-        QList<Collection*> m_collections;
+        QList<Collection*> m_collections{};
-        QHash<const DatabaseWidget*, Collection*> m_dbToCollection;
+        QHash<const DatabaseWidget*, Collection*> m_dbToCollection{};
 
-        QList<Session*> m_sessions;
+        QList<Session*> m_sessions{};
 
-        bool m_insideEnsureDefaultAlias;
+        bool m_insideEnsureDefaultAlias{false};
+        QSet<const DatabaseWidget*> m_unlockingDb{}; // list of db being unlocking
+        QSet<const DatabaseWidget*> m_lockingDb{}; // list of db being locking
     };
 
 } // namespace FdoSecrets

src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp

@@ -255,6 +255,7 @@ void SettingsWidgetFdoSecrets::loadSettings()
     m_ui->showNotification->setChecked(FdoSecrets::settings()->showNotification());
     m_ui->confirmDeleteItem->setChecked(FdoSecrets::settings()->confirmDeleteItem());
     m_ui->confirmAccessItem->setChecked(FdoSecrets::settings()->confirmAccessItem());
+    m_ui->unlockBeforeSearch->setChecked(FdoSecrets::settings()->unlockBeforeSearch());
 }
 
 void SettingsWidgetFdoSecrets::saveSettings()
@@ -263,6 +264,7 @@ void SettingsWidgetFdoSecrets::saveSettings()
     FdoSecrets::settings()->setShowNotification(m_ui->showNotification->isChecked());
     FdoSecrets::settings()->setConfirmDeleteItem(m_ui->confirmDeleteItem->isChecked());
     FdoSecrets::settings()->setConfirmAccessItem(m_ui->confirmAccessItem->isChecked());
+    FdoSecrets::settings()->setUnlockBeforeSearch(m_ui->unlockBeforeSearch->isChecked());
 }
 
 void SettingsWidgetFdoSecrets::showEvent(QShowEvent* event)

src/fdosecrets/widgets/SettingsWidgetFdoSecrets.ui

@@ -72,7 +72,11 @@
        <item>
         <widget class="QCheckBox" name="confirmDeleteItem">
          <property name="toolTip">
-          <string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;&lt;span style=&quot; font-family:'-apple-system','BlinkMacSystemFont','Segoe UI','Helvetica','Arial','sans-serif','Apple Color Emoji','Segoe UI Emoji'; font-size:14px; color:#24292e; background-color:#ffffff;&quot;&gt;This setting does not override disabling recycle bin prompts&lt;/span&gt;&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
+          <string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;&lt;span style=&quot;
+           font-family:'-apple-system','BlinkMacSystemFont','Segoe UI','Helvetica','Arial','sans-serif','Apple Color
+           Emoji','Segoe UI Emoji'; font-size:14px; color:#24292e; background-color:#ffffff;&quot;&gt;This setting does
+           not override disabling recycle bin prompts&lt;/span&gt;&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;
+          </string>
          </property>
          <property name="text">
           <string>Confirm when clients request entry deletion</string>
@@ -82,6 +86,23 @@
          </property>
         </widget>
        </item>
+       <item>
+        <widget class="QCheckBox" name="unlockBeforeSearch">
+         <property name="toolTip">
+          <string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;This improves compatibility with certain applications
+           which search for password without unlocking the database first.&lt;/p&gt;&lt;p&gt;But enabling this may also
+           crash the client if the database can not be unlocked within a certain timeout. (Usually 25s, but may be a
+           different value set in applications.)&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;
+          </string>
+         </property>
+         <property name="text">
+          <string>Prompt to unlock database before searching</string>
+         </property>
+         <property name="checked">
+          <bool>true</bool>
+         </property>
+        </widget>
+       </item>
        <item>
         <spacer name="verticalSpacer">
          <property name="orientation">

src/gui/GuiTools.cpp

@@ -66,10 +66,10 @@ namespace GuiTools
         }
     }
 
-    void deleteEntriesResolveReferences(QWidget* parent, const QList<Entry*>& entries, bool permanent)
+    size_t deleteEntriesResolveReferences(QWidget* parent, const QList<Entry*>& entries, bool permanent)
     {
         if (!parent || entries.isEmpty()) {
-            return;
+            return 0;
         }
 
         QList<Entry*> selectedEntries;
@@ -116,5 +116,6 @@ namespace GuiTools
                 entry->database()->recycleEntry(entry);
             }
         }
+        return selectedEntries.size();
     }
 } // namespace GuiTools

src/gui/GuiTools.h

@@ -26,6 +26,6 @@ class Entry;
 namespace GuiTools
 {
     bool confirmDeleteEntries(QWidget* parent, const QList<Entry*>& entries, bool permanent);
-    void deleteEntriesResolveReferences(QWidget* parent, const QList<Entry*>& entries, bool permanent);
+    size_t deleteEntriesResolveReferences(QWidget* parent, const QList<Entry*>& entries, bool permanent);
 } // namespace GuiTools
 #endif // KEEPASSXC_GUITOOLS_H

src/gui/entry/EditEntryWidget.cpp

@@ -69,20 +69,20 @@ EditEntryWidget::EditEntryWidget(QWidget* parent)
     , m_browserUi(new Ui::EditEntryWidgetBrowser())
     , m_attachments(new EntryAttachments())
     , m_customData(new CustomData())
-    , m_mainWidget(new QScrollArea())
+    , m_mainWidget(new QScrollArea(this))
-    , m_advancedWidget(new QWidget())
+    , m_advancedWidget(new QWidget(this))
-    , m_iconsWidget(new EditWidgetIcons())
+    , m_iconsWidget(new EditWidgetIcons(this))
-    , m_autoTypeWidget(new QWidget())
+    , m_autoTypeWidget(new QWidget(this))
 #ifdef WITH_XC_SSHAGENT
-    , m_sshAgentWidget(new QWidget())
+    , m_sshAgentWidget(new QWidget(this))
 #endif
 #ifdef WITH_XC_BROWSER
     , m_browserSettingsChanged(false)
-    , m_browserWidget(new QWidget())
+    , m_browserWidget(new QWidget(this))
     , m_additionalURLsDataModel(new EntryURLModel(this))
 #endif
-    , m_editWidgetProperties(new EditWidgetProperties())
+    , m_editWidgetProperties(new EditWidgetProperties(this))
-    , m_historyWidget(new QWidget())
+    , m_historyWidget(new QWidget(this))
     , m_entryAttributes(new EntryAttributes(this))
     , m_attributesModel(new EntryAttributesModel(m_advancedWidget))
     , m_historyModel(new EntryHistoryModel(this))

tests/gui/TestGuiFdoSecrets.cpp

@@ -186,7 +186,7 @@ void TestGuiFdoSecrets::init()
 
     // make sure window is activated or focus tests may fail
     m_mainWindow->activateWindow();
-    QApplication::processEvents();
+    processEvents();
 
     // open and unlock the database
     m_tabWidget->addDatabaseTab(m_dbFile->fileName(), false, "a");
@@ -202,6 +202,7 @@ void TestGuiFdoSecrets::init()
 void TestGuiFdoSecrets::cleanup()
 {
     // restore to default settings
+    FdoSecrets::settings()->setUnlockBeforeSearch(false);
     FdoSecrets::settings()->setShowNotification(false);
     FdoSecrets::settings()->setConfirmAccessItem(false);
     FdoSecrets::settings()->setEnabled(false);
@@ -213,8 +214,14 @@ void TestGuiFdoSecrets::cleanup()
     for (int i = 0; i != m_tabWidget->count(); ++i) {
         m_tabWidget->databaseWidgetFromIndex(i)->database()->markAsClean();
     }
+
+    // Close any dialogs
+    while (auto w = QApplication::activeModalWidget()) {
+        w->close();
+    }
+
     VERIFY(m_tabWidget->closeAllDatabaseTabs());
-    QApplication::processEvents();
+    processEvents();
 
     if (m_dbFile) {
         m_dbFile->remove();
@@ -250,7 +257,7 @@ void TestGuiFdoSecrets::testServiceEnable()
     VERIFY(sigError.isEmpty());
     COMPARE(sigStarted.size(), 1);
 
-    QApplication::processEvents();
+    processEvents();
 
     VERIFY(QDBusConnection::sessionBus().interface()->isServiceRegistered(DBUS_SERVICE_SECRET));
 
@@ -288,9 +295,11 @@ void TestGuiFdoSecrets::testServiceSearch()
     auto item = getFirstItem(coll);
     VERIFY(item);
 
-    auto entries = m_db->rootGroup()->entriesRecursive(false);
+    auto itemObj = m_plugin->dbus()->pathToObject<Item>(QDBusObjectPath(item->path()));
-    VERIFY(!entries.isEmpty());
+    VERIFY(itemObj);
-    const auto& entry = entries.first();
+    auto entry = itemObj->backend();
+    VERIFY(entry);
+
     entry->attributes()->set("fdosecrets-test", "1");
     entry->attributes()->set("fdosecrets-test-protected", "2", true);
     const QString crazyKey = "_a:bc&-+'-e%12df_d";
@@ -336,6 +345,72 @@ void TestGuiFdoSecrets::testServiceSearch()
     }
 }
 
+void TestGuiFdoSecrets::testServiceSearchBlockingUnlock()
+{
+    auto service = enableService();
+    VERIFY(service);
+    auto coll = getDefaultCollection(service);
+    VERIFY(coll);
+
+    auto entries = m_db->rootGroup()->entriesRecursive();
+    VERIFY(!entries.isEmpty());
+    // assumes the db is not empty
+    auto title = entries.first()->title();
+
+    // NOTE: entries are no longer valid after locking
+    lockDatabaseInBackend();
+
+    // when database is locked, nothing is returned
+    FdoSecrets::settings()->setUnlockBeforeSearch(false);
+    {
+        DBUS_GET2(unlocked, locked, service->SearchItems({{"Title", title}}));
+        COMPARE(locked, {});
+        COMPARE(unlocked, {});
+    }
+
+    // when database is locked, nothing is returned
+    FdoSecrets::settings()->setUnlockBeforeSearch(true);
+    {
+        // SearchItems will block because the blocking wait is implemented
+        // using a local QEventLoop.
+        // so we do a little trick here to get the return value back
+        bool unlockDialogWorks = false;
+        QTimer::singleShot(50, [&]() { unlockDialogWorks = driveUnlockDialog(); });
+
+        DBUS_GET2(unlocked, locked, service->SearchItems({{"Title", title}}));
+        VERIFY(unlockDialogWorks);
+        COMPARE(locked, {});
+        COMPARE(unlocked.size(), 1);
+        auto item = getProxy<ItemProxy>(unlocked.first());
+        DBUS_COMPARE(item->label(), title);
+    }
+}
+
+void TestGuiFdoSecrets::testServiceSearchForce()
+{
+    auto service = enableService();
+    VERIFY(service);
+    auto coll = getDefaultCollection(service);
+    VERIFY(coll);
+    auto item = getFirstItem(coll);
+    VERIFY(item);
+
+    auto itemObj = m_plugin->dbus()->pathToObject<Item>(QDBusObjectPath(item->path()));
+    VERIFY(itemObj);
+    auto entry = itemObj->backend();
+    VERIFY(entry);
+
+    // fdosecrets should still find the item even if searching is disabled
+    entry->group()->setSearchingEnabled(Group::Disable);
+
+    // search by title
+    {
+        DBUS_GET2(unlocked, locked, service->SearchItems({{"Title", entry->title()}}));
+        COMPARE(locked, {});
+        COMPARE(unlocked, {QDBusObjectPath(item->path())});
+    }
+}
+
 void TestGuiFdoSecrets::testServiceUnlock()
 {
     lockDatabaseInBackend();
@@ -362,46 +437,83 @@ void TestGuiFdoSecrets::testServiceUnlock()
     VERIFY(spyPromptCompleted.isValid());
 
     // nothing is unlocked yet
-    QTRY_COMPARE(spyPromptCompleted.count(), 0);
+    VERIFY(waitForSignal(spyPromptCompleted, 0));
     DBUS_COMPARE(coll->locked(), true);
 
-    // drive the prompt
+    // show the prompt
     DBUS_VERIFY(prompt->Prompt(""));
 
     // still not unlocked before user action
-    QTRY_COMPARE(spyPromptCompleted.count(), 0);
+    VERIFY(waitForSignal(spyPromptCompleted, 0));
     DBUS_COMPARE(coll->locked(), true);
 
-    // interact with the dialog
+    VERIFY(driveUnlockDialog());
-    QApplication::processEvents();
-    {
-        auto dbOpenDlg = m_tabWidget->findChild<DatabaseOpenDialog*>();
-        VERIFY(dbOpenDlg);
-        auto editPassword = dbOpenDlg->findChild<QLineEdit*>("editPassword");
-        VERIFY(editPassword);
-        editPassword->setFocus();
-        QTest::keyClicks(editPassword, "a");
-        QTest::keyClick(editPassword, Qt::Key_Enter);
-    }
-    QApplication::processEvents();
 
-    // unlocked
+    VERIFY(waitForSignal(spyPromptCompleted, 1));
-    DBUS_COMPARE(coll->locked(), false);
-
-    QTRY_COMPARE(spyPromptCompleted.count(), 1);
     {
         auto args = spyPromptCompleted.takeFirst();
         COMPARE(args.size(), 2);
         COMPARE(args.at(0).toBool(), false);
         COMPARE(getSignalVariantArgument<QList<QDBusObjectPath>>(args.at(1)), {QDBusObjectPath(coll->path())});
     }
-    QTRY_COMPARE(spyCollectionCreated.count(), 0);
+
+    // check unlocked *AFTER* the prompt signal
+    DBUS_COMPARE(coll->locked(), false);
+
+    VERIFY(waitForSignal(spyCollectionCreated, 0));
     QTRY_VERIFY(!spyCollectionChanged.isEmpty());
     for (const auto& args : spyCollectionChanged) {
         COMPARE(args.size(), 1);
         COMPARE(args.at(0).value<QDBusObjectPath>().path(), coll->path());
     }
-    QTRY_COMPARE(spyCollectionDeleted.count(), 0);
+    VERIFY(waitForSignal(spyCollectionDeleted, 0));
+}
+
+void TestGuiFdoSecrets::testServiceUnlockDatabaseConcurrent()
+{
+    lockDatabaseInBackend();
+
+    auto service = enableService();
+    VERIFY(service);
+    auto coll = getDefaultCollection(service);
+    VERIFY(coll);
+
+    DBUS_GET2(unlocked, promptPath, service->Unlock({QDBusObjectPath(coll->path())}));
+    auto prompt = getProxy<PromptProxy>(promptPath);
+    VERIFY(prompt);
+    QSignalSpy spyPromptCompleted(prompt.data(), SIGNAL(Completed(bool, QDBusVariant)));
+    VERIFY(spyPromptCompleted.isValid());
+    DBUS_VERIFY(prompt->Prompt(""));
+
+    // while the first prompt is running, another request come in
+    DBUS_GET2(unlocked2, promptPath2, service->Unlock({QDBusObjectPath(coll->path())}));
+    auto prompt2 = getProxy<PromptProxy>(promptPath2);
+    VERIFY(prompt2);
+    QSignalSpy spyPromptCompleted2(prompt2.data(), SIGNAL(Completed(bool, QDBusVariant)));
+    VERIFY(spyPromptCompleted2.isValid());
+    DBUS_VERIFY(prompt2->Prompt(""));
+
+    // there should be only one unlock dialog
+    VERIFY(driveUnlockDialog());
+
+    // both prompts should complete
+    VERIFY(waitForSignal(spyPromptCompleted, 1));
+    {
+        auto args = spyPromptCompleted.takeFirst();
+        COMPARE(args.size(), 2);
+        COMPARE(args.at(0).toBool(), false);
+        COMPARE(getSignalVariantArgument<QList<QDBusObjectPath>>(args.at(1)), {QDBusObjectPath(coll->path())});
+    }
+    VERIFY(waitForSignal(spyPromptCompleted2, 1));
+    {
+        auto args = spyPromptCompleted2.takeFirst();
+        COMPARE(args.size(), 2);
+        COMPARE(args.at(0).toBool(), false);
+        COMPARE(getSignalVariantArgument<QList<QDBusObjectPath>>(args.at(1)), {QDBusObjectPath(coll->path())});
+    }
+
+    // check unlocked *AFTER* prompt signal
+    DBUS_COMPARE(coll->locked(), false);
 }
 
 void TestGuiFdoSecrets::testServiceUnlockItems()
@@ -438,17 +550,16 @@ void TestGuiFdoSecrets::testServiceUnlockItems()
         // only allow once
         VERIFY(driveAccessControlDialog(false));
 
-        // unlocked
+        VERIFY(waitForSignal(spyPromptCompleted, 1));
-        DBUS_COMPARE(item->locked(), false);
-
-        VERIFY(spyPromptCompleted.wait());
-        COMPARE(spyPromptCompleted.count(), 1);
         {
             auto args = spyPromptCompleted.takeFirst();
             COMPARE(args.size(), 2);
             COMPARE(args.at(0).toBool(), false);
             COMPARE(getSignalVariantArgument<QList<QDBusObjectPath>>(args.at(1)), {QDBusObjectPath(item->path())});
         }
+
+        // unlocked
+        DBUS_COMPARE(item->locked(), false);
     }
 
     // access the secret should reset the locking state
@@ -477,17 +588,16 @@ void TestGuiFdoSecrets::testServiceUnlockItems()
         // only allow and remember
         VERIFY(driveAccessControlDialog(true));
 
-        // unlocked
+        VERIFY(waitForSignal(spyPromptCompleted, 1));
-        DBUS_COMPARE(item->locked(), false);
-
-        VERIFY(spyPromptCompleted.wait());
-        COMPARE(spyPromptCompleted.count(), 1);
         {
             auto args = spyPromptCompleted.takeFirst();
             COMPARE(args.size(), 2);
             COMPARE(args.at(0).toBool(), false);
             COMPARE(getSignalVariantArgument<QList<QDBusObjectPath>>(args.at(1)), {QDBusObjectPath(item->path())});
         }
+
+        // unlocked
+        DBUS_COMPARE(item->locked(), false);
     }
 
     // access the secret does not reset the locking state
@@ -524,15 +634,15 @@ void TestGuiFdoSecrets::testServiceLock()
         // prompt and click cancel
         MessageBox::setNextAnswer(MessageBox::Cancel);
         DBUS_VERIFY(prompt->Prompt(""));
-        QApplication::processEvents();
+        processEvents();
 
-        DBUS_COMPARE(coll->locked(), false);
+        VERIFY(waitForSignal(spyPromptCompleted, 1));
-
-        QTRY_COMPARE(spyPromptCompleted.count(), 1);
         auto args = spyPromptCompleted.takeFirst();
         COMPARE(args.count(), 2);
         COMPARE(args.at(0).toBool(), true);
         COMPARE(getSignalVariantArgument<QList<QDBusObjectPath>>(args.at(1)), {});
+
+        DBUS_COMPARE(coll->locked(), false);
     }
     {
         DBUS_GET2(locked, promptPath, service->Lock({QDBusObjectPath(coll->path())}));
@@ -545,24 +655,24 @@ void TestGuiFdoSecrets::testServiceLock()
         // prompt and click save
         MessageBox::setNextAnswer(MessageBox::Save);
         DBUS_VERIFY(prompt->Prompt(""));
-        QApplication::processEvents();
+        processEvents();
 
-        DBUS_COMPARE(coll->locked(), true);
+        VERIFY(waitForSignal(spyPromptCompleted, 1));
-
-        QTRY_COMPARE(spyPromptCompleted.count(), 1);
         auto args = spyPromptCompleted.takeFirst();
         COMPARE(args.count(), 2);
         COMPARE(args.at(0).toBool(), false);
         COMPARE(getSignalVariantArgument<QList<QDBusObjectPath>>(args.at(1)), {QDBusObjectPath(coll->path())});
+
+        DBUS_COMPARE(coll->locked(), true);
     }
 
-    QTRY_COMPARE(spyCollectionCreated.count(), 0);
+    VERIFY(waitForSignal(spyCollectionCreated, 0));
     QTRY_VERIFY(!spyCollectionChanged.isEmpty());
     for (const auto& args : spyCollectionChanged) {
         COMPARE(args.size(), 1);
         COMPARE(args.at(0).value<QDBusObjectPath>().path(), coll->path());
     }
-    QTRY_COMPARE(spyCollectionDeleted.count(), 0);
+    VERIFY(waitForSignal(spyCollectionDeleted, 0));
 
     // locking item locks the whole db
     unlockDatabaseInBackend();
@@ -575,12 +685,59 @@ void TestGuiFdoSecrets::testServiceLock()
 
         MessageBox::setNextAnswer(MessageBox::Save);
         DBUS_VERIFY(prompt->Prompt(""));
-        QApplication::processEvents();
+        processEvents();
 
         DBUS_COMPARE(coll->locked(), true);
     }
 }
 
+void TestGuiFdoSecrets::testServiceLockConcurrent()
+{
+    auto service = enableService();
+    VERIFY(service);
+    auto coll = getDefaultCollection(service);
+    VERIFY(coll);
+
+    m_db->markAsModified();
+
+    DBUS_GET2(locked, promptPath, service->Lock({QDBusObjectPath(coll->path())}));
+    auto prompt = getProxy<PromptProxy>(promptPath);
+    VERIFY(prompt);
+    QSignalSpy spyPromptCompleted(prompt.data(), SIGNAL(Completed(bool, QDBusVariant)));
+    VERIFY(spyPromptCompleted.isValid());
+
+    DBUS_GET2(locked2, promptPath2, service->Lock({QDBusObjectPath(coll->path())}));
+    auto prompt2 = getProxy<PromptProxy>(promptPath2);
+    VERIFY(prompt2);
+    QSignalSpy spyPromptCompleted2(prompt2.data(), SIGNAL(Completed(bool, QDBusVariant)));
+    VERIFY(spyPromptCompleted2.isValid());
+
+    // prompt and click save
+    MessageBox::setNextAnswer(MessageBox::Save);
+    DBUS_VERIFY(prompt->Prompt(""));
+
+    // second prompt should not show dialog
+    DBUS_VERIFY(prompt2->Prompt(""));
+
+    VERIFY(waitForSignal(spyPromptCompleted, 1));
+    {
+        auto args = spyPromptCompleted.takeFirst();
+        COMPARE(args.count(), 2);
+        COMPARE(args.at(0).toBool(), false);
+        COMPARE(getSignalVariantArgument<QList<QDBusObjectPath>>(args.at(1)), {QDBusObjectPath(coll->path())});
+    }
+
+    VERIFY(waitForSignal(spyPromptCompleted2, 1));
+    {
+        auto args = spyPromptCompleted2.takeFirst();
+        COMPARE(args.count(), 2);
+        COMPARE(args.at(0).toBool(), false);
+        COMPARE(getSignalVariantArgument<QList<QDBusObjectPath>>(args.at(1)), {QDBusObjectPath(coll->path())});
+    }
+
+    DBUS_COMPARE(coll->locked(), true);
+}
+
 void TestGuiFdoSecrets::testSessionOpen()
 {
     auto service = enableService();
@@ -621,7 +778,7 @@ void TestGuiFdoSecrets::testCollectionCreate()
         COMPARE(promptPath, QDBusObjectPath("/"));
         COMPARE(collPath.path(), existing->path());
     }
-    QTRY_COMPARE(spyCollectionCreated.count(), 0);
+    VERIFY(waitForSignal(spyCollectionCreated, 0));
 
     // create new one and set properties
     {
@@ -635,11 +792,10 @@ void TestGuiFdoSecrets::testCollectionCreate()
         QSignalSpy spyPromptCompleted(prompt.data(), SIGNAL(Completed(bool, QDBusVariant)));
         VERIFY(spyPromptCompleted.isValid());
 
-        QTimer::singleShot(50, this, &TestGuiFdoSecrets::driveNewDatabaseWizard);
         DBUS_VERIFY(prompt->Prompt(""));
-        QApplication::processEvents();
+        VERIFY(driveNewDatabaseWizard());
 
-        QTRY_COMPARE(spyPromptCompleted.count(), 1);
+        VERIFY(waitForSignal(spyPromptCompleted, 1));
         auto args = spyPromptCompleted.takeFirst();
         COMPARE(args.size(), 2);
         COMPARE(args.at(0).toBool(), false);
@@ -648,7 +804,7 @@ void TestGuiFdoSecrets::testCollectionCreate()
 
         DBUS_COMPARE(coll->label(), QStringLiteral("Test NewDB"));
 
-        QTRY_COMPARE(spyCollectionCreated.count(), 1);
+        VERIFY(waitForSignal(spyCollectionCreated, 1));
         {
             args = spyCollectionCreated.takeFirst();
             COMPARE(args.size(), 1);
@@ -657,34 +813,6 @@ void TestGuiFdoSecrets::testCollectionCreate()
     }
 }
 
-void TestGuiFdoSecrets::driveNewDatabaseWizard()
-{
-    auto wizard = m_tabWidget->findChild<NewDatabaseWizard*>();
-    VERIFY(wizard);
-
-    COMPARE(wizard->currentId(), 0);
-    wizard->next();
-    wizard->next();
-    COMPARE(wizard->currentId(), 2);
-
-    // enter password
-    auto* passwordEdit = wizard->findChild<QLineEdit*>("enterPasswordEdit");
-    auto* passwordRepeatEdit = wizard->findChild<QLineEdit*>("repeatPasswordEdit");
-    QTest::keyClicks(passwordEdit, "test");
-    QTest::keyClick(passwordEdit, Qt::Key::Key_Tab);
-    QTest::keyClicks(passwordRepeatEdit, "test");
-
-    // save database to temporary file
-    TemporaryFile tmpFile;
-    VERIFY(tmpFile.open());
-    tmpFile.close();
-    fileDialog()->setNextFileName(tmpFile.fileName());
-
-    wizard->accept();
-
-    tmpFile.remove();
-}
-
 void TestGuiFdoSecrets::testCollectionDelete()
 {
     auto service = enableService();
@@ -711,7 +839,12 @@ void TestGuiFdoSecrets::testCollectionDelete()
     // closing the tab should have deleted the database if not in testing
     // but deleteLater is not processed in QApplication::processEvent
     // see https://doc.qt.io/qt-5/qcoreapplication.html#processEvents
-    QApplication::processEvents();
+
+    VERIFY(waitForSignal(spyPromptCompleted, 1));
+    auto args = spyPromptCompleted.takeFirst();
+    COMPARE(args.count(), 2);
+    COMPARE(args.at(0).toBool(), false);
+    COMPARE(args.at(1).value<QDBusVariant>().variant().toString(), QStringLiteral(""));
 
     // however, the object should already be taken down from dbus
     {
@@ -720,13 +853,7 @@ void TestGuiFdoSecrets::testCollectionDelete()
         COMPARE(reply.error().type(), QDBusError::UnknownObject);
     }
 
-    QTRY_COMPARE(spyPromptCompleted.count(), 1);
+    VERIFY(waitForSignal(spyCollectionDeleted, 1));
-    auto args = spyPromptCompleted.takeFirst();
-    COMPARE(args.count(), 2);
-    COMPARE(args.at(0).toBool(), false);
-    COMPARE(args.at(1).value<QDBusVariant>().variant().toString(), QStringLiteral(""));
-
-    QTRY_COMPARE(spyCollectionDeleted.count(), 1);
     {
         args = spyCollectionDeleted.takeFirst();
         COMPARE(args.size(), 1);
@@ -734,6 +861,57 @@ void TestGuiFdoSecrets::testCollectionDelete()
     }
 }
 
+void TestGuiFdoSecrets::testCollectionDeleteConcurrent()
+{
+    auto service = enableService();
+    VERIFY(service);
+    auto coll = getDefaultCollection(service);
+    VERIFY(coll);
+
+    m_db->markAsModified();
+    DBUS_GET(promptPath, coll->Delete());
+    auto prompt = getProxy<PromptProxy>(promptPath);
+    VERIFY(prompt);
+    QSignalSpy spyPromptCompleted(prompt.data(), SIGNAL(Completed(bool, QDBusVariant)));
+    VERIFY(spyPromptCompleted.isValid());
+
+    // before interacting with the prompt, another request come in
+    DBUS_GET(promptPath2, coll->Delete());
+    auto prompt2 = getProxy<PromptProxy>(promptPath);
+    VERIFY(prompt2);
+    QSignalSpy spyPromptCompleted2(prompt2.data(), SIGNAL(Completed(bool, QDBusVariant)));
+    VERIFY(spyPromptCompleted2.isValid());
+
+    // prompt and click save
+    MessageBox::setNextAnswer(MessageBox::Save);
+    DBUS_VERIFY(prompt->Prompt(""));
+
+    // there should be no prompt
+    DBUS_VERIFY(prompt2->Prompt(""));
+
+    VERIFY(waitForSignal(spyPromptCompleted, 1));
+    {
+        auto args = spyPromptCompleted.takeFirst();
+        COMPARE(args.count(), 2);
+        COMPARE(args.at(0).toBool(), false);
+        COMPARE(args.at(1).value<QDBusVariant>().variant().toString(), QStringLiteral(""));
+    }
+
+    VERIFY(waitForSignal(spyPromptCompleted2, 1));
+    {
+        auto args = spyPromptCompleted2.takeFirst();
+        COMPARE(args.count(), 2);
+        COMPARE(args.at(0).toBool(), false);
+        COMPARE(args.at(1).value<QDBusVariant>().variant().toString(), QStringLiteral(""));
+    }
+
+    {
+        auto reply = coll->locked();
+        VERIFY(reply.isFinished() && reply.isError());
+        COMPARE(reply.error().type(), QDBusError::UnknownObject);
+    }
+}
+
 void TestGuiFdoSecrets::testCollectionChange()
 {
     auto service = enableService();
@@ -822,7 +1000,7 @@ void TestGuiFdoSecrets::testItemCreate()
 
     // signals
     {
-        QTRY_COMPARE(spyItemCreated.count(), 1);
+        VERIFY(waitForSignal(spyItemCreated, 1));
         auto args = spyItemCreated.takeFirst();
         COMPARE(args.size(), 1);
         COMPARE(args.at(0).value<QDBusObjectPath>().path(), item->path());
@@ -942,7 +1120,7 @@ void TestGuiFdoSecrets::testItemReplace()
         QSet<QDBusObjectPath> expected{QDBusObjectPath(item1->path()), QDBusObjectPath(item2->path())};
         COMPARE(QSet<QDBusObjectPath>::fromList(unlocked), expected);
 
-        QTRY_COMPARE(spyItemCreated.count(), 0);
+        VERIFY(waitForSignal(spyItemCreated, 0));
         // there may be multiple changed signals, due to each item attribute is set separately
         QTRY_VERIFY(!spyItemChanged.isEmpty());
         for (const auto& args : spyItemChanged) {
@@ -968,7 +1146,7 @@ void TestGuiFdoSecrets::testItemReplace()
         };
         COMPARE(QSet<QDBusObjectPath>::fromList(unlocked), expected);
 
-        QTRY_COMPARE(spyItemCreated.count(), 1);
+        VERIFY(waitForSignal(spyItemCreated, 1));
         {
             auto args = spyItemCreated.takeFirst();
             COMPARE(args.size(), 1);
@@ -1013,7 +1191,7 @@ void TestGuiFdoSecrets::testItemReplaceExistingLocked()
         DBUS_COMPARE(item->locked(), true);
     }
 
-    // when replace with a locked item, there will be an prompt
+    // when replace with a locked item, there will be a prompt
     auto item2 = createItem(sess, coll, "abc2", "PasswordUpdated", attr1, true, true);
     VERIFY(item2);
     COMPARE(item2->path(), item->path());
@@ -1061,7 +1239,7 @@ void TestGuiFdoSecrets::testItemSecret()
         COMPARE(ss.contentType, TEXT_PLAIN);
         COMPARE(ss.value, entry->password().toUtf8());
 
-        QTRY_COMPARE(spyShowNotification.count(), 1);
+        VERIFY(waitForSignal(spyShowNotification, 1));
     }
     FdoSecrets::settings()->setShowNotification(false);
 
@@ -1125,15 +1303,14 @@ void TestGuiFdoSecrets::testItemDelete()
     VERIFY(itemObj);
     MessageBox::setNextAnswer(MessageBox::Delete);
     DBUS_VERIFY(prompt->Prompt(""));
-    QApplication::processEvents();
 
-    QTRY_COMPARE(spyPromptCompleted.count(), 1);
+    VERIFY(waitForSignal(spyPromptCompleted, 1));
     auto args = spyPromptCompleted.takeFirst();
     COMPARE(args.count(), 2);
     COMPARE(args.at(0).toBool(), false);
     COMPARE(args.at(1).toString(), QStringLiteral(""));
 
-    QTRY_COMPARE(spyItemDeleted.count(), 1);
+    VERIFY(waitForSignal(spyItemDeleted, 1));
     args = spyItemDeleted.takeFirst();
     COMPARE(args.size(), 1);
     COMPARE(args.at(0).value<QDBusObjectPath>().path(), itemPath);
@@ -1275,7 +1452,7 @@ void TestGuiFdoSecrets::testModifyingExposedGroup()
 
     m_db->metadata()->setRecycleBinEnabled(true);
     m_db->recycleGroup(subgroup);
-    QApplication::processEvents();
+    processEvents();
 
     {
         DBUS_GET(collPaths, service->collections());
@@ -1284,7 +1461,7 @@ void TestGuiFdoSecrets::testModifyingExposedGroup()
 
     // test setting another exposed group, the collection will be exposed again
     FdoSecrets::settings()->setExposedGroup(m_db, m_db->rootGroup()->uuid());
-    QApplication::processEvents();
+    processEvents();
     {
         DBUS_GET(collPaths, service->collections());
         COMPARE(collPaths.size(), 1);
@@ -1295,14 +1472,25 @@ void TestGuiFdoSecrets::lockDatabaseInBackend()
 {
     m_dbWidget->lock();
     m_db.reset();
-    QApplication::processEvents();
+    processEvents();
 }
 
 void TestGuiFdoSecrets::unlockDatabaseInBackend()
 {
     m_dbWidget->performUnlockDatabase("a");
     m_db = m_dbWidget->database();
-    QApplication::processEvents();
+    processEvents();
+}
+
+void TestGuiFdoSecrets::processEvents()
+{
+    // Couldn't use QApplication::processEvents, because per Qt documentation:
+    //     events that are posted while the function runs will be queued until a later round of event processing.
+    // and we may post QTimer single shot events during event handling to achieve async method.
+    // So we directly call event dispatcher in a loop until no events can be handled
+    while (QAbstractEventDispatcher::instance()->processEvents(QEventLoop::AllEvents)) {
+        // pass
+    }
 }
 
 // the following functions have return value, switch macros to the version supporting that
@@ -1388,9 +1576,7 @@ QSharedPointer<ItemProxy> TestGuiFdoSecrets::createItem(const QSharedPointer<Ses
     bool found = driveAccessControlDialog();
     COMPARE(found, expectPrompt);
 
-    // wait for signal
+    VERIFY(waitForSignal(spyPromptCompleted, 1));
-    VERIFY(spyPromptCompleted.wait());
-    COMPARE(spyPromptCompleted.count(), 1);
     auto args = spyPromptCompleted.takeFirst();
     COMPARE(args.size(), 2);
     COMPARE(args.at(0).toBool(), false);
@@ -1401,24 +1587,94 @@ QSharedPointer<ItemProxy> TestGuiFdoSecrets::createItem(const QSharedPointer<Ses
 
 bool TestGuiFdoSecrets::driveAccessControlDialog(bool remember)
 {
-    QApplication::processEvents();
+    processEvents();
-    for (auto w : qApp->allWidgets()) {
+    for (auto w : QApplication::topLevelWidgets()) {
         if (!w->isWindow()) {
             continue;
         }
         auto dlg = qobject_cast<AccessControlDialog*>(w);
-        if (dlg) {
+        if (dlg && dlg->isVisible()) {
             auto rememberCheck = dlg->findChild<QCheckBox*>("rememberCheck");
             VERIFY(rememberCheck);
             rememberCheck->setChecked(remember);
-            QTest::keyClick(dlg, Qt::Key_Enter);
+            dlg->done(AccessControlDialog::AllowSelected);
-            QApplication::processEvents();
+            processEvents();
+            VERIFY(dlg->isHidden());
             return true;
         }
     }
     return false;
 }
 
+bool TestGuiFdoSecrets::driveNewDatabaseWizard()
+{
+    // processEvents will block because the NewDatabaseWizard is shown using exec
+    // which creates a local QEventLoop.
+    // so we do a little trick here to get the return value back
+    bool ret = false;
+    QTimer::singleShot(0, this, [this, &ret]() {
+        ret = [this]() -> bool {
+            auto wizard = m_tabWidget->findChild<NewDatabaseWizard*>();
+            VERIFY(wizard);
+
+            COMPARE(wizard->currentId(), 0);
+            wizard->next();
+            wizard->next();
+            COMPARE(wizard->currentId(), 2);
+
+            // enter password
+            auto* passwordEdit = wizard->findChild<QLineEdit*>("enterPasswordEdit");
+            auto* passwordRepeatEdit = wizard->findChild<QLineEdit*>("repeatPasswordEdit");
+            VERIFY(passwordEdit);
+            VERIFY(passwordRepeatEdit);
+            QTest::keyClicks(passwordEdit, "test");
+            QTest::keyClick(passwordEdit, Qt::Key::Key_Tab);
+            QTest::keyClicks(passwordRepeatEdit, "test");
+
+            // save database to temporary file
+            TemporaryFile tmpFile;
+            VERIFY(tmpFile.open());
+            tmpFile.close();
+            fileDialog()->setNextFileName(tmpFile.fileName());
+
+            wizard->accept();
+
+            tmpFile.remove();
+            return true;
+        }();
+    });
+    processEvents();
+    return ret;
+}
+
+bool TestGuiFdoSecrets::driveUnlockDialog()
+{
+    processEvents();
+    auto dbOpenDlg = m_tabWidget->findChild<DatabaseOpenDialog*>();
+    VERIFY(dbOpenDlg);
+    auto editPassword = dbOpenDlg->findChild<QLineEdit*>("editPassword");
+    VERIFY(editPassword);
+    editPassword->setFocus();
+    QTest::keyClicks(editPassword, "a");
+    QTest::keyClick(editPassword, Qt::Key_Enter);
+    processEvents();
+    return true;
+}
+
+bool TestGuiFdoSecrets::waitForSignal(QSignalSpy& spy, int expectedCount)
+{
+    processEvents();
+    // If already expected count, do not wait and return immediately
+    if (spy.count() == expectedCount) {
+        return true;
+    } else if (spy.count() > expectedCount) {
+        return false;
+    }
+    spy.wait();
+    COMPARE(spy.count(), expectedCount);
+    return true;
+}
+
 #undef VERIFY
 #define VERIFY QVERIFY
 #undef COMPARE

tests/gui/TestGuiFdoSecrets.h

@@ -23,6 +23,7 @@
 #include <QPointer>
 
 #include "fdosecrets/dbus/DBusTypes.h"
+#include "gui/MessageBox.h"
 
 class MainWindow;
 class Database;
@@ -47,6 +48,7 @@ class SessionProxy;
 class PromptProxy;
 
 class QAbstractItemView;
+class QSignalSpy;
 
 class TestGuiFdoSecrets : public QObject
 {
@@ -64,15 +66,20 @@ private slots:
     void testServiceEnable();
     void testServiceEnableNoExposedDatabase();
     void testServiceSearch();
+    void testServiceSearchBlockingUnlock();
+    void testServiceSearchForce();
     void testServiceUnlock();
+    void testServiceUnlockDatabaseConcurrent();
     void testServiceUnlockItems();
     void testServiceLock();
+    void testServiceLockConcurrent();
 
     void testSessionOpen();
     void testSessionClose();
 
     void testCollectionCreate();
     void testCollectionDelete();
+    void testCollectionDeleteConcurrent();
     void testCollectionChange();
 
     void testItemCreate();
@@ -92,11 +99,14 @@ private slots:
     void testHiddenFilename();
     void testDuplicateName();
 
-protected slots:
-    void driveNewDatabaseWizard();
-    bool driveAccessControlDialog(bool remember = true);
-
 private:
+    bool driveUnlockDialog();
+    bool driveNewDatabaseWizard();
+    bool driveAccessControlDialog(bool remember = true);
+    bool waitForSignal(QSignalSpy& spy, int expectedCount);
+
+    void processEvents();
+
     void lockDatabaseInBackend();
     void unlockDatabaseInBackend();
     QSharedPointer<ServiceProxy> enableService();