diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts
index 71529937c..ee955050a 100644
--- a/share/translations/keepassxc_en.ts
+++ b/share/translations/keepassxc_en.ts
@@ -7880,11 +7880,27 @@ Please consider generating a new key file.
- <html><head/><body><p><span style=" 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;">This setting does not override disabling recycle bin prompts</span></p></body></html>
+ Confirm when clients request entry deletion
- Confirm when clients request entry deletion
+ <html><head/><body><p><span style="
+ 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;">This setting does
+ not override disabling recycle bin prompts</span></p></body></html>
+
+
+
+
+ <html><head/><body><p>This improves compatibility with certain applications
+ which search for password without unlocking the database first.</p><p>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.)</p></body></html>
+
+
+
+
+ Prompt to unlock database before searching
diff --git a/src/core/Config.cpp b/src/core/Config.cpp
index b586cee05..54fc59c81 100644
--- a/src/core/Config.cpp
+++ b/src/core/Config.cpp
@@ -178,6 +178,7 @@ static const QHash 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}},
diff --git a/src/core/Config.h b/src/core/Config.h
index c0e3d142b..8e5c14e35 100644
--- a/src/core/Config.h
+++ b/src/core/Config.h
@@ -155,6 +155,7 @@ public:
FdoSecrets_ShowNotification,
FdoSecrets_ConfirmDeleteItem,
FdoSecrets_ConfirmAccessItem,
+ FdoSecrets_UnlockBeforeSearch,
KeeShare_QuietSuccess,
KeeShare_Own,
diff --git a/src/fdosecrets/FdoSecretsSettings.cpp b/src/fdosecrets/FdoSecretsSettings.cpp
index 58126733a..2f1e95182 100644
--- a/src/fdosecrets/FdoSecretsSettings.cpp
+++ b/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& db) const
{
return exposedGroup(db.data());
diff --git a/src/fdosecrets/FdoSecretsSettings.h b/src/fdosecrets/FdoSecretsSettings.h
index 24a37a8d2..31ab005f6 100644
--- a/src/fdosecrets/FdoSecretsSettings.h
+++ b/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& db) const;
diff --git a/src/fdosecrets/dbus/DBusMgr.cpp b/src/fdosecrets/dbus/DBusMgr.cpp
index e60c068aa..1a4b2b73c 100644
--- a/src/fdosecrets/dbus/DBusMgr.cpp
+++ b/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));
diff --git a/src/fdosecrets/dbus/DBusObject.h b/src/fdosecrets/dbus/DBusObject.h
index d2314438d..8984cf17e 100644
--- a/src/fdosecrets/dbus/DBusObject.h
+++ b/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))
{
}
diff --git a/src/fdosecrets/objects/Collection.cpp b/src/fdosecrets/objects/Collection.cpp
index c4e24839b..610a4b99e 100644
--- a/src/fdosecrets/objects/Collection.cpp
+++ b/src/fdosecrets/objects/Collection.cpp
@@ -24,7 +24,9 @@
#include "core/Tools.h"
#include "gui/DatabaseWidget.h"
+#include "gui/GuiTools.h"
+#include
#include
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(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& items)
+ DBusResult
+ Collection::searchItems(const DBusClientPtr& client, const StringStringMap& attributes, QList& 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(service(), QSet{this}, QSet{});
+ 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");
- 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();
-
- itemPath = attributes.value(ItemAttributes::PathKey);
-
- // check existing item using attributes
- QList existing;
- ret = searchItems(attributes, existing);
- if (ret.err()) {
- return ret;
- }
- if (!existing.isEmpty() && replace) {
- item = existing.front();
- }
- }
-
- prompt = PromptBase::Create(service(), this, properties, secret, itemPath, item);
+ prompt = PromptBase::Create(service(), this, properties, secret, replace);
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 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)
diff --git a/src/fdosecrets/objects/Collection.h b/src/fdosecrets/objects/Collection.h
index 690eb0d99..4bd83d152 100644
--- a/src/fdosecrets/objects/Collection.h
+++ b/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 searchItems(const StringStringMap& attributes, QList& items);
+ Q_INVOKABLE DBusResult remove(PromptBase*& prompt);
+ Q_INVOKABLE DBusResult searchItems(const DBusClientPtr& client,
+ const StringStringMap& attributes,
+ QList& 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
- void doDeleteEntries(QList entries);
+ bool doLock();
+ // 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();
diff --git a/src/fdosecrets/objects/Item.cpp b/src/fdosecrets/objects/Item.cpp
index e6c2831b4..9a7d94739 100644
--- a/src/fdosecrets/objects/Item.cpp
+++ b/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();
diff --git a/src/fdosecrets/objects/Item.h b/src/fdosecrets/objects/Item.h
index e11cdb88c..8be4e7d27 100644
--- a/src/fdosecrets/objects/Item.h
+++ b/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.
diff --git a/src/fdosecrets/objects/Prompt.cpp b/src/fdosecrets/objects/Prompt.cpp
index 9fa06c25e..58fdfa4e1 100644
--- a/src/fdosecrets/objects/Prompt.cpp
+++ b/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(parent());
}
- DBusResult PromptBase::dismiss()
- {
- emit completed(true, "");
- return {};
- }
-
- DeleteCollectionPrompt::DeleteCollectionPrompt(Service* parent, Collection* coll)
- : PromptBase(parent)
- , m_collection(coll)
- {
- }
-
- DBusResult DeleteCollectionPrompt::prompt(const DBusClientPtr&, const QString& windowId)
+ DBusResult PromptBase::prompt(const DBusClientPtr& client, const QString& windowId)
{
if (thread() != QThread::currentThread()) {
DBusResult ret;
@@ -81,19 +70,62 @@ namespace FdoSecrets
return ret;
}
- if (!m_collection) {
- return DBusResult(DBUS_ERROR_SECRET_NO_SUCH_OBJECT);
- }
-
- MessageBox::OverrideParent override(findWindow(windowId));
- // only need to delete in backend, collection will react itself.
- auto accepted = service()->doCloseDatabase(m_collection->backend());
-
- emit completed(!accepted, "");
-
+ QWeakPointer weak = client;
+ // execute the actual prompt method in event loop to avoid block this method
+ QTimer::singleShot(0, this, [this, weak, windowId]() {
+ auto c = weak.lock();
+ if (!c) {
+ return;
+ }
+ if (m_signalSent) {
+ 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()) {
- DBusResult ret;
- QMetaObject::invokeMethod(
- this, "prompt", Qt::BlockingQueuedConnection, Q_ARG(QString, windowId), Q_RETURN_ARG(DBusResult, ret));
- return ret;
- }
+ return QVariant::fromValue(DBusMgr::objectPathSafe(m_coll));
+ }
+ PromptResult CreateCollectionPrompt::promptSync(const DBusClientPtr&, const QString& windowId)
+ {
MessageBox::OverrideParent override(findWindow(windowId));
- auto coll = service()->doNewDatabase();
- if (!coll) {
- return dismiss();
- }
-
- auto ret = coll->setProperties(m_properties);
+ bool created = false;
+ // collection with the alias may be created since the prompt was created
+ auto ret = service()->readAlias(m_alias, m_coll);
if (ret.err()) {
- coll->doDelete();
- return dismiss();
+ return ret;
+ }
+ 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();
- return dismiss();
+ if (created) {
+ 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()) {
- DBusResult ret;
- QMetaObject::invokeMethod(
- this, "prompt", Qt::BlockingQueuedConnection, Q_ARG(QString, windowId), Q_RETURN_ARG(DBusResult, ret));
- return ret;
- }
+ return QVariant::fromValue(m_locked);
+ }
+ 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()) {
- return dismiss();
+ auto accepted = c->doLock();
+ if (accepted) {
+ m_locked << c->objectPath();
}
- m_locked << c->objectPath();
}
}
-
- emit completed(false, QVariant::fromValue(m_locked));
-
- return {};
- }
-
- DBusResult LockCollectionsPrompt::dismiss()
- {
- emit completed(true, QVariant::fromValue(m_locked));
- return {};
+ return PromptResult::accepted(m_locked.size() == m_collections.size());
}
UnlockPrompt::UnlockPrompt(Service* parent, const QSet& colls, const QSet& items)
@@ -194,15 +217,13 @@ namespace FdoSecrets
}
}
- DBusResult UnlockPrompt::prompt(const DBusClientPtr& client, const QString& windowId)
+ QVariant UnlockPrompt::currentResult() const
{
- if (thread() != QThread::currentThread()) {
- DBusResult ret;
- QMetaObject::invokeMethod(
- this, "prompt", Qt::BlockingQueuedConnection, Q_ARG(QString, windowId), Q_RETURN_ARG(DBusResult, ret));
- return ret;
- }
+ return QVariant::fromValue(m_unlocked);
+ }
+ 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
- QTimer::singleShot(0, this, &UnlockPrompt::unlockItems);
+ 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
- // didn't unlock.
+ // no longer need to unlock the item if its containing collection didn't unlock.
m_items.remove(coll);
}
@@ -270,7 +290,7 @@ namespace FdoSecrets
// flatten to list of entries
QList 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));
- }
-
- DBusResult UnlockPrompt::dismiss()
- {
- emit completed(true, QVariant::fromValue(m_unlocked));
- return {};
+ finishPrompt(m_numRejected > 0);
}
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();
}
-
- emit completed(false, "");
-
- return {};
+ return PromptResult::accepted(deleted);
}
CreateItemPrompt::CreateItemPrompt(Service* parent,
Collection* coll,
QVariantMap properties,
Secret secret,
- QString itemPath,
- Item* existing)
+ bool replace)
: PromptBase(parent)
, m_coll(coll)
, m_properties(std::move(properties))
, m_secret(std::move(secret))
- , m_itemPath(std::move(itemPath))
- , m_item(existing)
+ , m_replace(replace)
+ , 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()) {
- DBusResult ret;
- QMetaObject::invokeMethod(
- this, "prompt", Qt::BlockingQueuedConnection, Q_ARG(QString, windowId), Q_RETURN_ARG(DBusResult, ret));
- return ret;
+ return QVariant::fromValue(DBusMgr::objectPathSafe(m_item));
+ }
+
+ PromptResult CreateItemPrompt::promptSync(const DBusClientPtr& client, const QString& windowId)
+ {
+ if (!m_coll) {
+ return PromptResult::accepted(false);
}
- MessageBox::OverrideParent override(findWindow(windowId));
-
- if (!m_coll) {
- return dismiss();
+ bool locked = true;
+ auto ret = m_coll->locked(locked);
+ if (locked) {
+ // 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();
+
+ itemPath = attributes.value(ItemAttributes::PathKey);
+
+ // check existing item using attributes
+ QList 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();
- if (ret.err()) {
- m_item->doDelete();
- return ret;
+ // the item may be locked due to authorization
+ ret = m_item->locked(client, locked);
+ if (ret.err()) {
+ return ret;
+ }
+ if (locked) {
+ // give the user a chance to unlock the item
+ auto prompt = PromptBase::Create(service(), QSet{}, QSet{m_item});
+ if (!prompt) {
+ return DBusResult{QDBusError::InternalError};
}
- emit completed(false, QVariant::fromValue(m_item->objectPath()));
- } else {
- bool locked = false;
- auto ret = m_item->locked(client, locked);
+ // postpone anything after the confirmation
+ connect(prompt, &PromptBase::completed, this, [this]() {
+ auto res = updateItem();
+ finishPrompt(res.err());
+ });
+
+ ret = prompt->prompt(client, windowId);
if (ret.err()) {
return ret;
}
- if (locked) {
- // give the user a chance to unlock the item
- auto prompt = PromptBase::Create(service(), QSet{}, QSet{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 PromptResult::Pending;
}
- return {};
- }
- DBusResult CreateItemPrompt::dismiss()
- {
- emit completed(true, QVariant::fromValue(DBusMgr::objectPathSafe(nullptr)));
- return {};
- }
-
- void CreateItemPrompt::itemUnlocked(bool dismissed, const QVariant& result)
- {
- auto unlocked = result.value>();
- 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)));
+ // the item can be updated directly
+ return updateItem();
}
DBusResult CreateItemPrompt::updateItem()
diff --git a/src/fdosecrets/objects/Prompt.h b/src/fdosecrets/objects/Prompt.h
index e3de1916a..2e9ffed9c 100644
--- a/src/fdosecrets/objects/Prompt.h
+++ b/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 virtual DBusResult dismiss();
+ Q_INVOKABLE DBusResult prompt(const DBusClientPtr& client, const QString& windowId);
+ Q_INVOKABLE DBusResult dismiss();
template 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:
- DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
-
- private:
- friend class PromptBase;
+ PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
QPointer 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:
- DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
- DBusResult dismiss() override;
-
- private:
- friend class PromptBase;
+ PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
+ QVariant currentResult() const override;
QVariantMap m_properties;
QString m_alias;
+ Collection* m_coll{};
};
class LockCollectionsPrompt : public PromptBase
{
Q_OBJECT
+ friend class PromptBase;
explicit LockCollectionsPrompt(Service* parent, const QList& colls);
- public:
- DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
- DBusResult dismiss() override;
-
- private:
- friend class PromptBase;
+ PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
+ QVariant currentResult() const override;
QList> m_collections;
QList m_locked;
@@ -116,22 +161,17 @@ namespace FdoSecrets
class UnlockPrompt : public PromptBase
{
Q_OBJECT
+ friend class PromptBase;
explicit UnlockPrompt(Service* parent, const QSet& colls, const QSet& items);
- public:
- DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
- DBusResult dismiss() override;
+ PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
+ QVariant currentResult() const override;
- private slots:
void collectionUnlockFinished(bool accepted);
void itemUnlockFinished(const QHash& results);
-
- private:
void unlockItems();
- friend class PromptBase;
-
static constexpr auto FdoSecretsBackend = "FdoSecretsBackend";
QList> m_collections;
@@ -148,14 +188,11 @@ namespace FdoSecrets
class DeleteItemPrompt : public PromptBase
{
Q_OBJECT
+ friend class PromptBase;
explicit DeleteItemPrompt(Service* parent, Item* item);
- public:
- DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
-
- private:
- friend class PromptBase;
+ PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
QPointer- 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,
- Item* existing);
+ bool replace);
- public:
- DBusResult prompt(const DBusClientPtr& client, const QString& windowId) override;
- DBusResult dismiss() override;
- private slots:
- void itemUnlocked(bool dismissed, const QVariant& result);
+ PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
+ QVariant currentResult() const override;
- private:
DBusResult updateItem();
- friend class PromptBase;
-
QPointer m_coll;
QVariantMap m_properties;
Secret m_secret;
- QString m_itemPath;
+ bool m_replace;
+
QPointer
- m_item;
QPointer m_sess;
diff --git a/src/fdosecrets/objects/Service.cpp b/src/fdosecrets/objects/Service.cpp
index dc42b632e..60e057e7d 100644
--- a/src/fdosecrets/objects/Service.cpp
+++ b/src/fdosecrets/objects/Service.cpp
@@ -52,8 +52,10 @@ namespace FdoSecrets
, m_databases(std::move(dbTabs))
, m_insideEnsureDefaultAlias(false)
{
- connect(
- m_databases, &DatabaseTabWidget::databaseUnlockDialogFinished, this, &Service::doneUnlockDatabaseInDialog);
+ connect(m_databases,
+ &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 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(obj);
auto coll = item ? item->collection() : qobject_cast(obj);
// either way there should be a collection
@@ -317,6 +319,7 @@ namespace FdoSecrets
}
}
+ prompt = nullptr;
if (!collectionsToUnlock.isEmpty() || !itemsToUnlock.isEmpty()) {
prompt = PromptBase::Create(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
diff --git a/src/fdosecrets/objects/Service.h b/src/fdosecrets/objects/Service.h
index 84eee230d..422a3ca94 100644
--- a/src/fdosecrets/objects/Service.h
+++ b/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 dbWidget The unlocked the dbWidget if succeed
+ * @param accepted If false, the action is cancelled by the user
+ * @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;
- QPointer m_databases;
+ FdoSecretsPlugin* m_plugin{nullptr};
+ QPointer m_databases{};
- QHash m_aliases;
- QList m_collections;
- QHash m_dbToCollection;
+ QHash m_aliases{};
+ QList m_collections{};
+ QHash m_dbToCollection{};
- QList m_sessions;
+ QList m_sessions{};
- bool m_insideEnsureDefaultAlias;
+ bool m_insideEnsureDefaultAlias{false};
+ QSet m_unlockingDb{}; // list of db being unlocking
+ QSet m_lockingDb{}; // list of db being locking
};
} // namespace FdoSecrets
diff --git a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp
index 5ab22749b..842a40b98 100644
--- a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp
+++ b/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)
diff --git a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.ui b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.ui
index 7034d7bd6..bed666676 100644
--- a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.ui
+++ b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.ui
@@ -72,7 +72,11 @@
-
- <html><head/><body><p><span style=" 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;">This setting does not override disabling recycle bin prompts</span></p></body></html>
+ <html><head/><body><p><span style="
+ 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;">This setting does
+ not override disabling recycle bin prompts</span></p></body></html>
+
Confirm when clients request entry deletion
@@ -82,6 +86,23 @@
+ -
+
+
+ <html><head/><body><p>This improves compatibility with certain applications
+ which search for password without unlocking the database first.</p><p>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.)</p></body></html>
+
+
+
+ Prompt to unlock database before searching
+
+
+ true
+
+
+
-
diff --git a/src/gui/GuiTools.cpp b/src/gui/GuiTools.cpp
index 7aeeeb7b0..30963b374 100644
--- a/src/gui/GuiTools.cpp
+++ b/src/gui/GuiTools.cpp
@@ -66,10 +66,10 @@ namespace GuiTools
}
}
- void deleteEntriesResolveReferences(QWidget* parent, const QList& entries, bool permanent)
+ size_t deleteEntriesResolveReferences(QWidget* parent, const QList& entries, bool permanent)
{
if (!parent || entries.isEmpty()) {
- return;
+ return 0;
}
QList selectedEntries;
@@ -116,5 +116,6 @@ namespace GuiTools
entry->database()->recycleEntry(entry);
}
}
+ return selectedEntries.size();
}
} // namespace GuiTools
diff --git a/src/gui/GuiTools.h b/src/gui/GuiTools.h
index 14a54ab75..814537382 100644
--- a/src/gui/GuiTools.h
+++ b/src/gui/GuiTools.h
@@ -26,6 +26,6 @@ class Entry;
namespace GuiTools
{
bool confirmDeleteEntries(QWidget* parent, const QList& entries, bool permanent);
- void deleteEntriesResolveReferences(QWidget* parent, const QList& entries, bool permanent);
+ size_t deleteEntriesResolveReferences(QWidget* parent, const QList& entries, bool permanent);
} // namespace GuiTools
#endif // KEEPASSXC_GUITOOLS_H
diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp
index e736efaad..f9527966b 100644
--- a/src/gui/entry/EditEntryWidget.cpp
+++ b/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_advancedWidget(new QWidget())
- , m_iconsWidget(new EditWidgetIcons())
- , m_autoTypeWidget(new QWidget())
+ , m_mainWidget(new QScrollArea(this))
+ , m_advancedWidget(new QWidget(this))
+ , m_iconsWidget(new EditWidgetIcons(this))
+ , 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_historyWidget(new QWidget())
+ , m_editWidgetProperties(new EditWidgetProperties(this))
+ , m_historyWidget(new QWidget(this))
, m_entryAttributes(new EntryAttributes(this))
, m_attributesModel(new EntryAttributesModel(m_advancedWidget))
, m_historyModel(new EntryHistoryModel(this))
diff --git a/tests/gui/TestGuiFdoSecrets.cpp b/tests/gui/TestGuiFdoSecrets.cpp
index 5e8bd01cd..ffdcd338f 100644
--- a/tests/gui/TestGuiFdoSecrets.cpp
+++ b/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);
- VERIFY(!entries.isEmpty());
- const auto& entry = entries.first();
+ auto itemObj = m_plugin->dbus()->pathToObject
- (QDBusObjectPath(item->path()));
+ VERIFY(itemObj);
+ 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(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
- (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
- QApplication::processEvents();
- {
- auto dbOpenDlg = m_tabWidget->findChild();
- VERIFY(dbOpenDlg);
- auto editPassword = dbOpenDlg->findChild("editPassword");
- VERIFY(editPassword);
- editPassword->setFocus();
- QTest::keyClicks(editPassword, "a");
- QTest::keyClick(editPassword, Qt::Key_Enter);
- }
- QApplication::processEvents();
+ VERIFY(driveUnlockDialog());
- // unlocked
- DBUS_COMPARE(coll->locked(), false);
-
- QTRY_COMPARE(spyPromptCompleted.count(), 1);
+ VERIFY(waitForSignal(spyPromptCompleted, 1));
{
auto args = spyPromptCompleted.takeFirst();
COMPARE(args.size(), 2);
COMPARE(args.at(0).toBool(), false);
COMPARE(getSignalVariantArgument>(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().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(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(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>(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>(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
- DBUS_COMPARE(item->locked(), false);
-
- VERIFY(spyPromptCompleted.wait());
- COMPARE(spyPromptCompleted.count(), 1);
+ VERIFY(waitForSignal(spyPromptCompleted, 1));
{
auto args = spyPromptCompleted.takeFirst();
COMPARE(args.size(), 2);
COMPARE(args.at(0).toBool(), false);
COMPARE(getSignalVariantArgument>(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
- DBUS_COMPARE(item->locked(), false);
-
- VERIFY(spyPromptCompleted.wait());
- COMPARE(spyPromptCompleted.count(), 1);
+ VERIFY(waitForSignal(spyPromptCompleted, 1));
{
auto args = spyPromptCompleted.takeFirst();
COMPARE(args.size(), 2);
COMPARE(args.at(0).toBool(), false);
COMPARE(getSignalVariantArgument>(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);
-
- QTRY_COMPARE(spyPromptCompleted.count(), 1);
+ VERIFY(waitForSignal(spyPromptCompleted, 1));
auto args = spyPromptCompleted.takeFirst();
COMPARE(args.count(), 2);
COMPARE(args.at(0).toBool(), true);
COMPARE(getSignalVariantArgument>(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);
-
- QTRY_COMPARE(spyPromptCompleted.count(), 1);
+ VERIFY(waitForSignal(spyPromptCompleted, 1));
auto args = spyPromptCompleted.takeFirst();
COMPARE(args.count(), 2);
COMPARE(args.at(0).toBool(), false);
COMPARE(getSignalVariantArgument>(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().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(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(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>(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>(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();
- VERIFY(wizard);
-
- COMPARE(wizard->currentId(), 0);
- wizard->next();
- wizard->next();
- COMPARE(wizard->currentId(), 2);
-
- // enter password
- auto* passwordEdit = wizard->findChild("enterPasswordEdit");
- auto* passwordRepeatEdit = wizard->findChild("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().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);
- auto args = spyPromptCompleted.takeFirst();
- COMPARE(args.count(), 2);
- COMPARE(args.at(0).toBool(), false);
- COMPARE(args.at(1).value().variant().toString(), QStringLiteral(""));
-
- QTRY_COMPARE(spyCollectionDeleted.count(), 1);
+ VERIFY(waitForSignal(spyCollectionDeleted, 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(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(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().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().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().path(), item->path());
@@ -942,7 +1120,7 @@ void TestGuiFdoSecrets::testItemReplace()
QSet expected{QDBusObjectPath(item1->path()), QDBusObjectPath(item2->path())};
COMPARE(QSet::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::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().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 TestGuiFdoSecrets::createItem(const QSharedPointer TestGuiFdoSecrets::createItem(const QSharedPointerallWidgets()) {
+ processEvents();
+ for (auto w : QApplication::topLevelWidgets()) {
if (!w->isWindow()) {
continue;
}
auto dlg = qobject_cast(w);
- if (dlg) {
+ if (dlg && dlg->isVisible()) {
auto rememberCheck = dlg->findChild("rememberCheck");
VERIFY(rememberCheck);
rememberCheck->setChecked(remember);
- QTest::keyClick(dlg, Qt::Key_Enter);
- QApplication::processEvents();
+ dlg->done(AccessControlDialog::AllowSelected);
+ 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();
+ VERIFY(wizard);
+
+ COMPARE(wizard->currentId(), 0);
+ wizard->next();
+ wizard->next();
+ COMPARE(wizard->currentId(), 2);
+
+ // enter password
+ auto* passwordEdit = wizard->findChild("enterPasswordEdit");
+ auto* passwordRepeatEdit = wizard->findChild("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();
+ VERIFY(dbOpenDlg);
+ auto editPassword = dbOpenDlg->findChild("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
diff --git a/tests/gui/TestGuiFdoSecrets.h b/tests/gui/TestGuiFdoSecrets.h
index a53da4088..1ed6d7f66 100644
--- a/tests/gui/TestGuiFdoSecrets.h
+++ b/tests/gui/TestGuiFdoSecrets.h
@@ -23,6 +23,7 @@
#include
#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 enableService();