Git-Mirrors/keepassxc
1
0
Fork 0
mirror of https://github.com/keepassxreboot/keepassxc.git synced 2024-10-01 01:26:01 -04:00
Code Issues Packages Projects Releases Wiki Activity

FdoSecrets: add smarter handling of database unlock requests

Browse Source 
This commit implements the following logic:
* If there're already unlocked collections, just use those,
* otherwise, show the unlock dialog until there's an unlocked and exposed collection.

* Fixes #7574
This commit is contained in:
Aetf 2022-05-07 03:38:10 -04:00 committed by Jonathan White
parent 8711d31f24
commit 07755c324a
6 changed files with 151 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/fdosecrets/objects/Collection.cpp
View File

@ -210,8 +210,7 @@ namespace FdoSecrets
return {};
}
DBusResult
Collection::searchItems(const DBusClientPtr& client, const StringStringMap& attributes, QList<Item*>& items)
DBusResult Collection::searchItems(const DBusClientPtr&, const StringStringMap& attributes, QList<Item*>& items)
{
items.clear();
@ -220,24 +219,6 @@ namespace FdoSecrets
return ret;
}
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,

76
src/fdosecrets/objects/Service.cpp
View File

@ -184,6 +184,30 @@ namespace FdoSecrets
return {};
}
DBusResult Service::unlockedCollections(QList<Collection*>& unlocked) const
{
auto ret = collections(unlocked);
if (ret.err()) {
return ret;
}
// filter out locked collections
auto it = unlocked.begin();
while (it != unlocked.end()) {
bool isLocked = true;
ret = (*it)->locked(isLocked);
if (ret.err()) {
return ret;
}
if (isLocked) {
it = unlocked.erase(it);
} else {
++it;
}
}
return {};
}
DBusResult Service::openSession(const DBusClientPtr& client,
const QString& algorithm,
const QVariant& input,
@ -242,15 +266,43 @@ namespace FdoSecrets
DBusResult Service::searchItems(const DBusClientPtr& client,
const StringStringMap& attributes,
QList<Item*>& unlocked,
QList<Item*>& locked) const
QList<Item*>& locked)
{
QList<Collection*> colls;
auto ret = collections(colls);
// we can only search unlocked collections
QList<Collection*> unlockedColls;
auto ret = unlockedCollections(unlockedColls);
if (ret.err()) {
return ret;
}
for (const auto& coll : asConst(colls)) {
while (unlockedColls.isEmpty() && settings()->unlockBeforeSearch()) {
// enable compatibility mode by making sure at least one database is unlocked
QEventLoop loop;
bool wasAccepted = false;
connect(this, &Service::doneUnlockDatabaseInDialog, &loop, [&](bool accepted) {
wasAccepted = accepted;
loop.quit();
});
doUnlockAnyDatabaseInDialog();
// blocking wait
loop.exec();
if (!wasAccepted) {
// user cancelled, do not proceed
qWarning() << "user cancelled";
return {};
}
// need to recompute this because collections may disappear while in event loop
ret = unlockedCollections(unlockedColls);
if (ret.err()) {
return ret;
}
}
for (const auto& coll : asConst(unlockedColls)) {
QList<Item*> items;
ret = coll->searchItems(client, attributes, items);
if (ret.err()) {
@ -524,7 +576,7 @@ namespace FdoSecrets
}
// check if the db is already being unlocked to prevent multiple dialogs for the same db
if (m_unlockingDb.contains(dbWidget)) {
if (m_unlockingAnyDatabase || m_unlockingDb.contains(dbWidget)) {
return;
}
@ -536,21 +588,33 @@ namespace FdoSecrets
m_databases->unlockDatabaseInDialog(dbWidget, DatabaseOpenDialog::Intent::None);
}
void Service::doUnlockAnyDatabaseInDialog()
{
if (m_unlockingAnyDatabase || !m_unlockingDb.isEmpty()) {
return;
}
m_unlockingAnyDatabase = true;
m_databases->unlockAnyDatabaseInDialog(DatabaseOpenDialog::Intent::None);
}
void Service::onDatabaseUnlockDialogFinished(bool accepted, DatabaseWidget* dbWidget)
{
if (!m_unlockingDb.contains(dbWidget)) {
if (!m_unlockingAnyDatabase && !m_unlockingDb.contains(dbWidget)) {
// not our concern
return;
}
if (!accepted) {
emit doneUnlockDatabaseInDialog(false, dbWidget);
m_unlockingAnyDatabase = false;
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);
m_unlockingAnyDatabase = false;
disconnect(m_unlockingDb.take(dbWidget));
});
m_unlockingDb[dbWidget] = conn;

11
src/fdosecrets/objects/Service.h
View File

@ -65,7 +65,7 @@ namespace FdoSecrets
Q_INVOKABLE DBusResult searchItems(const DBusClientPtr& client,
const StringStringMap& attributes,
QList<Item*>& unlocked,
QList<Item*>& locked) const;
QList<Item*>& locked);
Q_INVOKABLE DBusResult unlock(const DBusClientPtr& client,
const QList<DBusObject*>& objects,
@ -125,6 +125,12 @@ namespace FdoSecrets
*/
void doUnlockDatabaseInDialog(DatabaseWidget* dbWidget);
/**
* Async, connect to signal doneUnlockDatabaseInDialog for finish notification
* @param dbWidget
*/
void doUnlockAnyDatabaseInDialog();
private slots:
void ensureDefaultAlias();
@ -154,6 +160,8 @@ namespace FdoSecrets
*/
Collection* findCollection(const DatabaseWidget* db) const;
DBusResult unlockedCollections(QList<Collection*>& unlocked) const;
private:
FdoSecretsPlugin* m_plugin{nullptr};
QPointer<DatabaseTabWidget> m_databases{};
@ -165,6 +173,7 @@ namespace FdoSecrets
QList<Session*> m_sessions{};
bool m_insideEnsureDefaultAlias{false};
bool m_unlockingAnyDatabase{false};
// list of db currently has unlock dialog shown
QHash<const DatabaseWidget*, QMetaObject::Connection> m_unlockingDb{};
QSet<const DatabaseWidget*> m_lockingDb{}; // list of db being locking

2
src/gui/DatabaseTabWidget.h
View File

@ -75,6 +75,7 @@ public slots:
void closeDatabaseFromSender();
void unlockDatabaseInDialog(DatabaseWidget* dbWidget, DatabaseOpenDialog::Intent intent);
void unlockDatabaseInDialog(DatabaseWidget* dbWidget, DatabaseOpenDialog::Intent intent, const QString& filePath);
void unlockAnyDatabaseInDialog(DatabaseOpenDialog::Intent intent);
void relockPendingDatabase();
void showDatabaseSecurity();
@ -106,7 +107,6 @@ private:
QSharedPointer<Database> execNewDatabaseWizard();
void updateLastDatabases(const QString& filename);
bool warnOnExport();
void unlockAnyDatabaseInDialog(DatabaseOpenDialog::Intent intent);
void displayUnlockDialog();
QPointer<DatabaseWidgetStateSync> m_dbWidgetStateSync;

70
tests/gui/TestGuiFdoSecrets.cpp
View File

@ -183,7 +183,7 @@ void TestGuiFdoSecrets::init()
m_dbFile.reset(new TemporaryFile());
// Write the temp storage to a temp database file for use in our tests
VERIFY(m_dbFile->open());
COMPARE(m_dbFile->write(m_dbData), static_cast<qint64>((m_dbData.size())));
COMPARE(m_dbFile->write(m_dbData), static_cast<qint64>(m_dbData.size()));
m_dbFile->close();
// make sure window is activated or focus tests may fail
@ -198,6 +198,12 @@ void TestGuiFdoSecrets::init()
// by default expose the root group
FdoSecrets::settings()->setExposedGroup(m_db, m_db->rootGroup()->uuid());
VERIFY(m_dbWidget->save());
// enforce consistent default settings at the beginning
FdoSecrets::settings()->setUnlockBeforeSearch(false);
FdoSecrets::settings()->setShowNotification(false);
FdoSecrets::settings()->setConfirmAccessItem(false);
FdoSecrets::settings()->setEnabled(false);
}
// Every test ends with closing the temp database without saving
@ -388,6 +394,62 @@ void TestGuiFdoSecrets::testServiceSearchBlockingUnlock()
}
}
void TestGuiFdoSecrets::testServiceSearchBlockingUnlockMultiple()
{
// setup: two databases, both locked, one with exposed db, the other not.
// add another database tab with a database with no exposed group
// to avoid modify the original, copy to a temp file first
QFile sourceDbFile(QStringLiteral(KEEPASSX_TEST_DATA_DIR "/NewDatabase2.kdbx"));
QByteArray dbData;
VERIFY(sourceDbFile.open(QIODevice::ReadOnly));
VERIFY(Tools::readAllFromDevice(&sourceDbFile, dbData));
sourceDbFile.close();
QTemporaryFile anotherFile;
VERIFY(anotherFile.open());
COMPARE(anotherFile.write(dbData), static_cast<qint64>(dbData.size()));
anotherFile.close();
m_tabWidget->addDatabaseTab(anotherFile.fileName(), false);
auto anotherWidget = m_tabWidget->currentDatabaseWidget();
auto service = enableService();
VERIFY(service);
// when there are multiple locked databases,
// repeatly show the dialog until there is at least one unlocked collection
FdoSecrets::settings()->setUnlockBeforeSearch(true);
// when only unlocking the one with no exposed group, a second dialog is shown
lockDatabaseInBackend();
{
bool unlockDialogWorks = false;
QTimer::singleShot(50, [&]() {
unlockDialogWorks = driveUnlockDialog(anotherWidget);
QTimer::singleShot(50, [&]() { unlockDialogWorks &= driveUnlockDialog(); });
});
DBUS_GET2(unlocked, locked, service->SearchItems({{"Title", "Sample Entry"}}));
VERIFY(unlockDialogWorks);
COMPARE(locked, {});
COMPARE(unlocked.size(), 1);
}
// when unlocking the one with exposed group, the other one remains locked
lockDatabaseInBackend();
{
bool unlockDialogWorks = false;
QTimer::singleShot(50, [&]() { unlockDialogWorks = driveUnlockDialog(m_dbWidget); });
DBUS_GET2(unlocked, locked, service->SearchItems({{"Title", "Sample Entry"}}));
VERIFY(unlockDialogWorks);
COMPARE(locked, {});
COMPARE(unlocked.size(), 1);
VERIFY(anotherWidget->isLocked());
}
}
void TestGuiFdoSecrets::testServiceSearchForce()
{
auto service = enableService();
@ -1625,7 +1687,7 @@ void TestGuiFdoSecrets::testNoExposeRecycleBin()
void TestGuiFdoSecrets::lockDatabaseInBackend()
{
m_dbWidget->lock();
m_tabWidget->lockDatabases();
m_db.reset();
processEvents();
}
@ -1825,7 +1887,7 @@ bool TestGuiFdoSecrets::driveNewDatabaseWizard()
return ret;
}
bool TestGuiFdoSecrets::driveUnlockDialog()
bool TestGuiFdoSecrets::driveUnlockDialog(DatabaseWidget* target)
{
processEvents();
auto dbOpenDlg = m_tabWidget->findChild<DatabaseOpenDialog*>();
@ -1833,6 +1895,8 @@ bool TestGuiFdoSecrets::driveUnlockDialog()
if (!dbOpenDlg->isVisible()) {
return false;
}
dbOpenDlg->setActiveDatabaseTab(target);
auto editPassword = dbOpenDlg->findChild<PasswordWidget*>("editPassword")->findChild<QLineEdit*>("passwordEdit");
VERIFY(editPassword);
editPassword->setFocus();

3
tests/gui/TestGuiFdoSecrets.h
View File

@ -67,6 +67,7 @@ private slots:
void testServiceEnableNoExposedDatabase();
void testServiceSearch();
void testServiceSearchBlockingUnlock();
void testServiceSearchBlockingUnlockMultiple();
void testServiceSearchForce();
void testServiceUnlock();
void testServiceUnlockDatabaseConcurrent();
@ -104,7 +105,7 @@ private slots:
void testDuplicateName();
private:
bool driveUnlockDialog();
bool driveUnlockDialog(DatabaseWidget* target = nullptr);
bool driveNewDatabaseWizard();
bool driveAccessControlDialog(bool remember = true, bool includeFutureEntries = false);
bool waitForSignal(QSignalSpy& spy, int expectedCount);