diff --git a/src/fdosecrets/objects/Prompt.cpp b/src/fdosecrets/objects/Prompt.cpp index 1ac7e306f..bd01de89c 100644 --- a/src/fdosecrets/objects/Prompt.cpp +++ b/src/fdosecrets/objects/Prompt.cpp @@ -210,7 +210,6 @@ namespace FdoSecrets m_collections.reserve(colls.size()); for (const auto& coll : asConst(colls)) { m_collections << coll; - connect(coll, &Collection::doneUnlockCollection, this, &UnlockPrompt::collectionUnlockFinished); } for (const auto& item : asConst(items)) { m_items[item->collection()] << item; @@ -234,6 +233,7 @@ namespace FdoSecrets bool waitingForCollections = false; for (const auto& c : asConst(m_collections)) { if (c) { + connect(c, &Collection::doneUnlockCollection, this, &UnlockPrompt::collectionUnlockFinished); // 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(); @@ -242,7 +242,7 @@ namespace FdoSecrets } // unlock items directly if no collection unlocking pending - // o.w. do it in collectionUnlockFinished + // o.w. doing it in collectionUnlockFinished if (!waitingForCollections) { unlockItems(); } @@ -400,18 +400,49 @@ namespace FdoSecrets return PromptResult::accepted(false); } - 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; + // give the user a chance to unlock the collection + // UnlockPrompt will handle the case of collection already unlocked + auto prompt = PromptBase::Create(service(), QSet{m_coll.data()}, QSet{}); + if (!prompt) { + return DBusResult{QDBusError::InternalError}; + } + // postpone anything after the prompt + connect(prompt, &PromptBase::completed, this, [this, windowId](bool dismissed) { + if (dismissed) { + finishPrompt(dismissed); + } else { + auto res = createItem(windowId); + if (res.err()) { + qWarning() << "FdoSecrets:" << res; + finishPrompt(true); + } + } + }); + + auto ret = prompt->prompt(client, windowId); + if (ret.err()) { + return ret; + } + return PromptResult::Pending; + } + + DBusResult CreateItemPrompt::createItem(const QString& windowId) + { + auto client = m_client.lock(); + if (!client) { + // client already gone + return {}; + } + + if (!m_coll) { + return DBusResult{DBUS_ERROR_SECRET_NO_SUCH_OBJECT}; + } + // get itemPath to create item and - // try finding an existing item using attributes + // try to find an existing item using attributes QString itemPath{}; auto iterAttr = m_properties.find(DBUS_INTERFACE_SECRET_ITEM + ".Attributes"); if (iterAttr != m_properties.end()) { @@ -425,7 +456,7 @@ namespace FdoSecrets // check existing item using attributes QList existing; - ret = m_coll->searchItems(client, attributes, existing); + auto ret = m_coll->searchItems(client, attributes, existing); if (ret.err()) { return ret; } @@ -444,31 +475,29 @@ namespace FdoSecrets } // the item may be locked due to authorization - ret = m_item->locked(client, 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}; + } + // postpone anything after the confirmation + connect(prompt, &PromptBase::completed, this, [this](bool dismissed) { + if (dismissed) { + finishPrompt(dismissed); + } else { + auto res = updateItem(); + if (res.err()) { + qWarning() << "FdoSecrets:" << res; + finishPrompt(true); + } + } + }); + + auto 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 DBusResult{QDBusError::InternalError}; - } - // 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; - } - return PromptResult::Pending; - } - - // the item can be updated directly - return updateItem(); + return {}; } DBusResult CreateItemPrompt::updateItem() @@ -493,6 +522,9 @@ namespace FdoSecrets if (ret.err()) { return ret; } + + // finally can finish the prompt without dismissing it + finishPrompt(false); return {}; } } // namespace FdoSecrets diff --git a/src/fdosecrets/objects/Prompt.h b/src/fdosecrets/objects/Prompt.h index 1727f4424..078e4ce86 100644 --- a/src/fdosecrets/objects/Prompt.h +++ b/src/fdosecrets/objects/Prompt.h @@ -211,6 +211,7 @@ namespace FdoSecrets PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override; QVariant currentResult() const override; + DBusResult createItem(const QString& windowId); DBusResult updateItem(); QPointer m_coll; diff --git a/src/fdosecrets/objects/Service.cpp b/src/fdosecrets/objects/Service.cpp index 8cc138362..d9b54710c 100644 --- a/src/fdosecrets/objects/Service.cpp +++ b/src/fdosecrets/objects/Service.cpp @@ -523,18 +523,37 @@ namespace FdoSecrets return; } - // mark the db as being unlocked to prevent multiple dialogs for the same db + // check if the db is already being unlocked to prevent multiple dialogs for the same db if (m_unlockingDb.contains(dbWidget)) { return; } - m_unlockingDb.insert(dbWidget); + // insert a dummy one here, just to prevent multiple dialogs + // the real one will be inserted in onDatabaseUnlockDialogFinished + m_unlockingDb[dbWidget] = {}; + + // actually show the dialog m_databases->unlockDatabaseInDialog(dbWidget, DatabaseOpenDialog::Intent::None); } void Service::onDatabaseUnlockDialogFinished(bool accepted, DatabaseWidget* dbWidget) { - m_unlockingDb.remove(dbWidget); - emit doneUnlockDatabaseInDialog(accepted, dbWidget); + if (!m_unlockingDb.contains(dbWidget)) { + // not our concern + return; + } + + if (!accepted) { + emit doneUnlockDatabaseInDialog(false, dbWidget); + m_unlockingDb.remove(dbWidget); + } else { + // delay the done signal to when the database is actually done with unlocking + // this is a oneshot connection to prevent superfluous signals + auto conn = connect(dbWidget, &DatabaseWidget::databaseUnlocked, this, [dbWidget, this]() { + emit doneUnlockDatabaseInDialog(true, dbWidget); + disconnect(m_unlockingDb.take(dbWidget)); + }); + m_unlockingDb[dbWidget] = conn; + } } } // namespace FdoSecrets diff --git a/src/fdosecrets/objects/Service.h b/src/fdosecrets/objects/Service.h index 422a3ca94..022023cae 100644 --- a/src/fdosecrets/objects/Service.h +++ b/src/fdosecrets/objects/Service.h @@ -128,6 +128,7 @@ namespace FdoSecrets private slots: void ensureDefaultAlias(); + void onDatabaseUnlockDialogFinished(bool accepted, DatabaseWidget* dbWidget); void onDatabaseTabOpened(DatabaseWidget* dbWidget, bool emitSignal); void monitorDatabaseExposedGroup(DatabaseWidget* dbWidget); @@ -136,8 +137,6 @@ namespace FdoSecrets void onCollectionAliasRemoved(const QString& alias); - void onDatabaseUnlockDialogFinished(bool accepted, DatabaseWidget* dbWidget); - private: bool initialize(); @@ -166,7 +165,8 @@ namespace FdoSecrets QList m_sessions{}; bool m_insideEnsureDefaultAlias{false}; - QSet m_unlockingDb{}; // list of db being unlocking + // list of db currently has unlock dialog shown + QHash m_unlockingDb{}; QSet m_lockingDb{}; // list of db being locking }; diff --git a/tests/gui/TestGuiFdoSecrets.cpp b/tests/gui/TestGuiFdoSecrets.cpp index ec9a16b5c..656ea135f 100644 --- a/tests/gui/TestGuiFdoSecrets.cpp +++ b/tests/gui/TestGuiFdoSecrets.cpp @@ -1094,6 +1094,31 @@ void TestGuiFdoSecrets::testItemCreate() } } +void TestGuiFdoSecrets::testItemCreateUnlock() +{ + auto service = enableService(); + VERIFY(service); + auto coll = getDefaultCollection(service); + VERIFY(coll); + auto sess = openSession(service, DhIetf1024Sha256Aes128CbcPkcs7::Algorithm); + VERIFY(sess); + + // NOTE: entries are no longer valid after locking + lockDatabaseInBackend(); + + QSignalSpy spyItemCreated(coll.data(), SIGNAL(ItemCreated(QDBusObjectPath))); + VERIFY(spyItemCreated.isValid()); + + // create item + StringStringMap attributes{ + {"application", "fdosecrets-test"}, + {"attr-i[bute]", "![some] -value*"}, + }; + + auto item = createItem(sess, coll, "abc", "Password", attributes, false, false, true); + VERIFY(item); +} + void TestGuiFdoSecrets::testItemChange() { auto service = enableService(); @@ -1678,7 +1703,8 @@ QSharedPointer TestGuiFdoSecrets::createItem(const QSharedPointer TestGuiFdoSecrets::createItem(const QSharedPointerPrompt("")); + + bool unlockFound = driveUnlockDialog(); + COMPARE(unlockFound, expectUnlockPrompt); + bool found = driveAccessControlDialog(); COMPARE(found, expectPrompt); @@ -1800,6 +1830,9 @@ bool TestGuiFdoSecrets::driveUnlockDialog() processEvents(); auto dbOpenDlg = m_tabWidget->findChild(); VERIFY(dbOpenDlg); + if (!dbOpenDlg->isVisible()) { + return false; + } auto editPassword = dbOpenDlg->findChild("editPassword")->findChild("passwordEdit"); VERIFY(editPassword); editPassword->setFocus(); diff --git a/tests/gui/TestGuiFdoSecrets.h b/tests/gui/TestGuiFdoSecrets.h index ab0909b30..9a43b78a3 100644 --- a/tests/gui/TestGuiFdoSecrets.h +++ b/tests/gui/TestGuiFdoSecrets.h @@ -84,6 +84,7 @@ private slots: void testCollectionChange(); void testItemCreate(); + void testItemCreateUnlock(); void testItemChange(); void testItemReplace(); void testItemReplaceExistingLocked(); @@ -122,7 +123,8 @@ private: const QString& pass, const FdoSecrets::wire::StringStringMap& attr, bool replace, - bool expectPrompt = false); + bool expectPrompt = false, + bool expectUnlockPrompt = false); FdoSecrets::wire::Secret encryptPassword(QByteArray value, QString contentType, const QSharedPointer& sess); template QSharedPointer getProxy(const QDBusObjectPath& path) const