Revert async Access Confirm Dialog

src/browser/BrowserAccessControlDialog.cpp

@@ -32,30 +32,16 @@ BrowserAccessControlDialog::BrowserAccessControlDialog(QWidget* parent)
 
     m_ui->setupUi(this);
 
-    connect(m_ui->allowButton, SIGNAL(clicked()), SLOT(acceptSelections()));
+    connect(m_ui->allowButton, SIGNAL(clicked()), SLOT(accept()));
-    connect(m_ui->denyButton, SIGNAL(clicked()), SLOT(rejectSelections()));
+    connect(m_ui->denyButton, SIGNAL(clicked()), SLOT(reject()));
-    connect(this, SIGNAL(rejected()), this, SIGNAL(closed()));
 }
 
 BrowserAccessControlDialog::~BrowserAccessControlDialog()
 {
 }
 
-void BrowserAccessControlDialog::closeEvent(QCloseEvent* event)
+void BrowserAccessControlDialog::setItems(const QList<Entry*>& items, const QString& urlString, bool httpAuth)
 {
-    // Emits closed signal when clicking X from title bar
-    emit closed();
-    event->accept();
-}
-
-void BrowserAccessControlDialog::setItems(const QList<Entry*>& entriesToConfirm,
-                                          const QList<Entry*>& allowedEntries,
-                                          const QString& urlString,
-                                          bool httpAuth)
-{
-    m_entriesToConfirm = entriesToConfirm;
-    m_allowedEntries = allowedEntries;
-
     QUrl url(urlString);
     m_ui->siteLabel->setText(m_ui->siteLabel->text().arg(
         url.toDisplayString(QUrl::RemoveUserInfo | QUrl::RemovePath | QUrl::RemoveQuery | QUrl::RemoveFragment)));
@@ -63,11 +49,11 @@ void BrowserAccessControlDialog::setItems(const QList<Entry*>& entriesToConfirm,
     m_ui->rememberDecisionCheckBox->setVisible(!httpAuth);
     m_ui->rememberDecisionCheckBox->setChecked(false);
 
-    m_ui->itemsTable->setRowCount(entriesToConfirm.count());
+    m_ui->itemsTable->setRowCount(items.count());
     m_ui->itemsTable->setColumnCount(2);
 
     int row = 0;
-    for (const auto& entry : entriesToConfirm) {
+    for (const auto& entry : items) {
         auto item = new QTableWidgetItem();
         item->setText(entry->title() + " - " + entry->username());
         item->setData(Qt::UserRole, row);
@@ -77,23 +63,18 @@ void BrowserAccessControlDialog::setItems(const QList<Entry*>& entriesToConfirm,
 
         auto disableButton = new QPushButton(tr("Disable for this site"));
         disableButton->setAutoDefault(false);
-
         connect(disableButton, &QAbstractButton::pressed, [&, item] {
             emit disableAccess(item);
             m_ui->itemsTable->removeRow(item->row());
-
             if (m_ui->itemsTable->rowCount() == 0) {
-                emit closed();
+                reject();
             }
         });
         m_ui->itemsTable->setCellWidget(row, 1, disableButton);
-
         ++row;
     }
-
     m_ui->itemsTable->resizeColumnsToContents();
     m_ui->itemsTable->horizontalHeader()->setSectionResizeMode(0, QHeaderView::Stretch);
-
     m_ui->allowButton->setFocus();
 }
 
@@ -102,11 +83,6 @@ bool BrowserAccessControlDialog::remember() const
     return m_ui->rememberDecisionCheckBox->isChecked();
 }
 
-bool BrowserAccessControlDialog::entriesAccepted() const
-{
-    return m_entriesAccepted;
-}
-
 QList<QTableWidgetItem*> BrowserAccessControlDialog::getSelectedEntries() const
 {
     QList<QTableWidgetItem*> selected;
@@ -130,19 +106,3 @@ QList<QTableWidgetItem*> BrowserAccessControlDialog::getNonSelectedEntries() con
     }
     return notSelected;
 }
-
-void BrowserAccessControlDialog::acceptSelections()
-{
-    auto selectedEntries = getSelectedEntries();
-
-    m_entriesAccepted = true;
-    emit acceptEntries(selectedEntries, m_entriesToConfirm, m_allowedEntries);
-    emit closed();
-}
-
-void BrowserAccessControlDialog::rejectSelections()
-{
-    auto rejectedEntries = getNonSelectedEntries();
-    emit rejectEntries(rejectedEntries, m_entriesToConfirm);
-    emit closed();
-}

src/browser/BrowserAccessControlDialog.h

@@ -37,28 +37,14 @@ public:
     explicit BrowserAccessControlDialog(QWidget* parent = nullptr);
     ~BrowserAccessControlDialog() override;
 
-    void setItems(const QList<Entry*>& entriesToConfirm,
+    void setItems(const QList<Entry*>& items, const QString& urlString, bool httpAuth);
-                  const QList<Entry*>& allowedEntries,
-                  const QString& urlString,
-                  bool httpAuth);
     bool remember() const;
-    bool entriesAccepted() const;
 
     QList<QTableWidgetItem*> getSelectedEntries() const;
     QList<QTableWidgetItem*> getNonSelectedEntries() const;
 
 signals:
     void disableAccess(QTableWidgetItem* item);
-    void acceptEntries(QList<QTableWidgetItem*> items, QList<Entry*> entriesToConfirm, QList<Entry*> allowedEntries);
-    void rejectEntries(QList<QTableWidgetItem*> items, QList<Entry*> entriesToConfirm);
-    void closed();
-
-public slots:
-    void acceptSelections();
-    void rejectSelections();
-
-private:
-    void closeEvent(QCloseEvent* event) override;
 
 private:
     QScopedPointer<Ui::BrowserAccessControlDialog> m_ui;

src/browser/BrowserAction.cpp

@@ -76,7 +76,7 @@ QJsonObject BrowserAction::handleAction(QLocalSocket* socket, const QJsonObject&
     } else if (action.compare("test-associate") == 0) {
         return handleTestAssociate(json, action);
     } else if (action.compare("get-logins") == 0) {
-        return handleGetLogins(socket, json, action);
+        return handleGetLogins(json, action);
     } else if (action.compare("generate-password") == 0) {
         return handleGeneratePassword(socket, json, action);
     } else if (action.compare("set-login") == 0) {
@@ -231,7 +231,7 @@ QJsonObject BrowserAction::handleTestAssociate(const QJsonObject& json, const QS
     return buildResponse(action, message, newNonce);
 }
 
-QJsonObject BrowserAction::handleGetLogins(QLocalSocket* socket, const QJsonObject& json, const QString& action)
+QJsonObject BrowserAction::handleGetLogins(const QJsonObject& json, const QString& action)
 {
     const QString hash = browserService()->getDatabaseHash();
     const QString nonce = json.value("nonce").toString();
@@ -264,31 +264,19 @@ QJsonObject BrowserAction::handleGetLogins(QLocalSocket* socket, const QJsonObje
     const QString formUrl = decrypted.value("submitUrl").toString();
     const QString auth = decrypted.value("httpAuth").toString();
     const bool httpAuth = auth.compare(TRUE_STR) == 0;
-    auto requestId = decrypted.value("requestID").toString();
 
-    if (browserService()->isAccessConfirmRequested()) {
+    const QJsonArray users = browserService()->findMatchingEntries(id, siteUrl, formUrl, "", keyList, httpAuth);
-        auto errorReply = getErrorReply(action, ERROR_KEEPASS_ACTION_CANCELLED_OR_DENIED);
+    if (users.isEmpty()) {
-
+        return getErrorReply(action, ERROR_KEEPASS_NO_LOGINS_FOUND);
-        if (!requestId.isEmpty()) {
-            errorReply["requestID"] = requestId;
-        }
-
-        return errorReply;
     }
 
-    browserService()->findEntries(socket,
+    QJsonObject message = browserMessageBuilder()->buildMessage(incrementedNonce);
-                                  incrementedNonce,
+    message["count"] = users.count();
-                                  m_clientPublicKey,
+    message["entries"] = users;
-                                  m_secretKey,
+    message["hash"] = hash;
-                                  id,
+    message["id"] = id;
-                                  hash,
+
-                                  requestId,
+    return buildResponse(action, message, incrementedNonce);
-                                  siteUrl,
-                                  formUrl,
-                                  "",
-                                  keyList,
-                                  httpAuth);
-    return QJsonObject();
 }
 
 QJsonObject BrowserAction::handleGeneratePassword(QLocalSocket* socket, const QJsonObject& json, const QString& action)

src/browser/BrowserAction.h

@@ -37,7 +37,7 @@ private:
     QJsonObject handleGetDatabaseHash(const QJsonObject& json, const QString& action);
     QJsonObject handleAssociate(const QJsonObject& json, const QString& action);
     QJsonObject handleTestAssociate(const QJsonObject& json, const QString& action);
-    QJsonObject handleGetLogins(QLocalSocket* socket, const QJsonObject& json, const QString& action);
+    QJsonObject handleGetLogins(const QJsonObject& json, const QString& action);
     QJsonObject handleGeneratePassword(QLocalSocket* socket, const QJsonObject& json, const QString& action);
     QJsonObject handleSetLogin(const QJsonObject& json, const QString& action);
     QJsonObject handleLockDatabase(const QJsonObject& json, const QString& action);

src/browser/BrowserService.cpp

@@ -1,6 +1,5 @@
 /*
  *  Copyright (C) 2013 Francois Ferrand
- *  Copyright (C) 2017 Sami Vänttinen <sami.vanttinen@protonmail.com>
  *  Copyright (C) 2022 KeePassXC Team <team@keepassxc.org>
  *
  *  This program is free software: you can redistribute it and/or modify
@@ -64,9 +63,9 @@ Q_GLOBAL_STATIC(BrowserService, s_browserService);
 BrowserService::BrowserService()
     : QObject()
     , m_browserHost(new BrowserHost)
+    , m_dialogActive(false)
     , m_bringToFrontRequested(false)
     , m_passwordGeneratorRequested(false)
-    , m_accessConfirmRequested(false)
     , m_prevWindowState(WindowState::Normal)
     , m_keepassBrowserUUID(Tools::hexToUuid("de887cc3036343b8974b5911b8816224"))
 {
@@ -314,18 +313,12 @@ QString BrowserService::getCurrentTotp(const QString& uuid)
     return {};
 }
 
-void BrowserService::findEntries(QLocalSocket* socket,
+QJsonArray BrowserService::findMatchingEntries(const QString& dbid,
-                                 const QString& nonce,
+                                               const QString& siteUrl,
-                                 const QString& publicKey,
+                                               const QString& formUrl,
-                                 const QString& secretKey,
+                                               const QString& realm,
-                                 const QString& dbid,
+                                               const StringPairList& keyList,
-                                 const QString& hash,
+                                               const bool httpAuth)
-                                 const QString& requestId,
-                                 const QString& siteUrl,
-                                 const QString& formUrl,
-                                 const QString& realm,
-                                 const StringPairList& keyList,
-                                 const bool httpAuth)
 {
     Q_UNUSED(dbid);
     const bool alwaysAllowAccess = browserSettings()->alwaysAllowAccess();
@@ -334,8 +327,8 @@ void BrowserService::findEntries(QLocalSocket* socket,
     const QString formHost = QUrl(formUrl).host();
 
     // Check entries for authorization
-    QList<Entry*> entriesToConfirm;
+    QList<Entry*> pwEntriesToConfirm;
-    QList<Entry*> allowedEntries;
+    QList<Entry*> pwEntries;
     for (auto* entry : searchEntries(siteUrl, formUrl, keyList)) {
         auto entryCustomData = entry->customData();
 
@@ -355,7 +348,7 @@ void BrowserService::findEntries(QLocalSocket* socket,
 
         // HTTP Basic Auth always needs a confirmation
         if (!ignoreHttpAuth && httpAuth) {
-            entriesToConfirm.append(entry);
+            pwEntriesToConfirm.append(entry);
             continue;
         }
 
@@ -365,166 +358,87 @@ void BrowserService::findEntries(QLocalSocket* socket,
 
         case Unknown:
             if (alwaysAllowAccess) {
-                allowedEntries.append(entry);
+                pwEntries.append(entry);
             } else {
-                entriesToConfirm.append(entry);
+                pwEntriesToConfirm.append(entry);
             }
             break;
 
         case Allowed:
-            allowedEntries.append(entry);
+            pwEntries.append(entry);
             break;
         }
     }
 
-    if (entriesToConfirm.isEmpty()) {
+    // Confirm entries
-        sendCredentialsToClient(allowedEntries, socket, nonce, publicKey, secretKey, hash, dbid, siteUrl, formUrl);
+    QList<Entry*> selectedEntriesToConfirm =
-        return;
+        confirmEntries(pwEntriesToConfirm, siteUrl, siteHost, formHost, realm, httpAuth);
+    if (!selectedEntriesToConfirm.isEmpty()) {
+        pwEntries.append(selectedEntriesToConfirm);
     }
 
-    confirmEntries(socket,
+    if (pwEntries.isEmpty()) {
-                   nonce,
+        return {};
-                   publicKey,
-                   secretKey,
-                   dbid,
-                   hash,
-                   requestId,
-                   allowedEntries,
-                   entriesToConfirm,
-                   siteUrl,
-                   siteHost,
-                   formHost,
-                   realm,
-                   httpAuth);
-}
-
-void BrowserService::confirmEntries(QLocalSocket* socket,
-                                    const QString& incrementedNonce,
-                                    const QString& publicKey,
-                                    const QString& secretKey,
-                                    const QString& id,
-                                    const QString& hash,
-                                    const QString& requestId,
-                                    QList<Entry*>& allowedEntries,
-                                    QList<Entry*>& entriesToConfirm,
-                                    const QString& siteUrl,
-                                    const QString& siteHost,
-                                    const QString& formUrl,
-                                    const QString& realm,
-                                    const bool httpAuth)
-{
-    if (entriesToConfirm.isEmpty() || m_accessConfirmRequested) {
-        return;
     }
 
-    if (!m_accessControlDialog) {
+    // Ensure that database is not locked when the popup was visible
-
+    if (!isDatabaseOpened()) {
-        m_accessControlDialog.reset(new BrowserAccessControlDialog());
+        return {};
-
-        connect(
-            m_currentDatabaseWidget, SIGNAL(databaseLockRequested()), m_accessControlDialog.data(), SIGNAL(closed()));
-
-        connect(m_accessControlDialog.data(),
-                &BrowserAccessControlDialog::disableAccess,
-                m_accessControlDialog.data(),
-                [=](QTableWidgetItem* item) {
-                    auto entry = entriesToConfirm[item->row()];
-                    denyEntry(entry, siteHost, formUrl, realm);
-                });
-
-        connect(m_accessControlDialog.data(), &BrowserAccessControlDialog::closed, m_accessControlDialog.data(), [=] {
-            if (!m_accessControlDialog->entriesAccepted()) {
-                auto errorMessage =
-                    browserMessageBuilder()->getErrorReply("get-logins", ERROR_KEEPASS_ACTION_CANCELLED_OR_DENIED);
-                errorMessage["requestID"] = requestId;
-                m_browserHost->sendClientMessage(socket, errorMessage);
-            }
-
-            m_accessControlDialog.reset();
-            hideWindow();
-            m_accessConfirmRequested = false;
-        });
-
-        connect(m_accessControlDialog.data(),
-                &BrowserAccessControlDialog::acceptEntries,
-                m_accessControlDialog.data(),
-                [=](QList<QTableWidgetItem*> items, QList<Entry*> entries, QList<Entry*> allowed) {
-                    QList<Entry*> selectedEntries;
-
-                    for (auto item : items) {
-                        auto entry = entries[item->row()];
-                        if (m_accessControlDialog->remember()) {
-                            allowEntry(entry, siteHost, formUrl, realm);
-                        }
-
-                        selectedEntries.append(entry);
-                    }
-
-                    hideWindow();
-
-                    if (!selectedEntries.isEmpty()) {
-                        allowed.append(selectedEntries);
-                    }
-
-                    if (allowed.isEmpty()) {
-                        return;
-                    }
-
-                    // Ensure that database is not locked when the popup was visible
-                    if (!isDatabaseOpened()) {
-                        return;
-                    }
-
-                    sendCredentialsToClient(
-                        allowed, socket, incrementedNonce, publicKey, secretKey, hash, id, siteUrl, formUrl);
-                });
-
-        connect(m_accessControlDialog.data(),
-                &BrowserAccessControlDialog::rejectEntries,
-                m_accessControlDialog.data(),
-                [=](QList<QTableWidgetItem*> items, QList<Entry*> entries) {
-                    Q_UNUSED(items); // We might need this later if single entries can be denied from the list
-                    for (auto entry : entries) {
-                        if (m_accessControlDialog->remember()) {
-                            denyEntry(entry, siteHost, formUrl, realm);
-                        }
-                    }
-                });
-
-        m_accessControlDialog->setItems(entriesToConfirm, allowedEntries, siteUrl, httpAuth);
-        m_accessControlDialog->show();
     }
 
-    m_accessConfirmRequested = true;
+    // Sort results
-    updateWindowState();
+    pwEntries = sortEntries(pwEntries, siteUrl, formUrl);
-}
-
-void BrowserService::sendCredentialsToClient(QList<Entry*>& allowedEntries,
-                                             QLocalSocket* socket,
-                                             const QString& incrementedNonce,
-                                             const QString& publicKey,
-                                             const QString& secretKey,
-                                             const QString& hash,
-                                             const QString& id,
-                                             const QString siteUrl,
-                                             const QString& formUrl)
-{
-    allowedEntries = sortEntries(allowedEntries, siteUrl, formUrl);
 
+    // Fill the list
     QJsonArray result;
-    for (auto* entry : allowedEntries) {
+    for (auto* entry : pwEntries) {
         result.append(prepareEntry(entry));
     }
 
-    QJsonObject message = browserMessageBuilder()->buildMessage(incrementedNonce);
+    return result;
-    message["count"] = result.count();
+}
-    message["entries"] = result;
-    message["hash"] = hash;
-    message["id"] = id;
 
-    m_browserHost->sendClientMessage(
+QList<Entry*> BrowserService::confirmEntries(QList<Entry*>& pwEntriesToConfirm,
-        socket, browserMessageBuilder()->buildResponse("get-logins", message, incrementedNonce, publicKey, secretKey));
+                                             const QString& siteUrl,
+                                             const QString& siteHost,
+                                             const QString& formUrl,
+                                             const QString& realm,
+                                             const bool httpAuth)
+{
+    if (pwEntriesToConfirm.isEmpty() || m_dialogActive) {
+        return {};
+    }
+
+    m_dialogActive = true;
+    updateWindowState();
+    BrowserAccessControlDialog accessControlDialog;
+
+    connect(m_currentDatabaseWidget, SIGNAL(databaseLockRequested()), &accessControlDialog, SLOT(reject()));
+
+    connect(&accessControlDialog, &BrowserAccessControlDialog::disableAccess, [&](QTableWidgetItem* item) {
+        auto entry = pwEntriesToConfirm[item->row()];
+        denyEntry(entry, siteHost, formUrl, realm);
+    });
+
+    accessControlDialog.setItems(pwEntriesToConfirm, siteUrl, httpAuth);
+
+    QList<Entry*> allowedEntries;
+    auto ret = accessControlDialog.exec();
+    if (ret == QDialog::Accepted) {
+        for (auto item : accessControlDialog.getSelectedEntries()) {
+            auto entry = pwEntriesToConfirm[item->row()];
+            if (accessControlDialog.remember()) {
+                allowEntry(entry, siteHost, formUrl, realm);
+            }
+            allowedEntries.append(entry);
+        }
+    }
+
+    // Re-hide the application if it wasn't visible before
     hideWindow();
+    m_dialogActive = false;
+
+    return allowedEntries;
 }
 
 void BrowserService::showPasswordGenerator(QLocalSocket* socket,
@@ -572,11 +486,6 @@ bool BrowserService::isPasswordGeneratorRequested() const
     return m_passwordGeneratorRequested;
 }
 
-bool BrowserService::isAccessConfirmRequested() const
-{
-    return m_accessConfirmRequested;
-}
-
 QString BrowserService::storeKey(const QString& key)
 {
     auto db = getDatabase();

src/browser/BrowserService.h

@@ -1,6 +1,5 @@
 /*
  *  Copyright (C) 2013 Francois Ferrand
- *  Copyright (C) 2017 Sami Vänttinen <sami.vanttinen@protonmail.com>
  *  Copyright (C) 2022 KeePassXC Team <team@keepassxc.org>
  *
  *  This program is free software: you can redistribute it and/or modify
@@ -63,9 +62,7 @@ public:
                                const QString& nonce,
                                const QString& publicKey,
                                const QString& secretKey);
-    void sendPassword(QLocalSocket* socket, const QJsonObject& message);
     bool isPasswordGeneratorRequested() const;
-    bool isAccessConfirmRequested() const;
 
     void addEntry(const QString& dbid,
                   const QString& login,
@@ -84,18 +81,12 @@ public:
                      const QString& siteUrl,
                      const QString& formUrl);
     bool deleteEntry(const QString& uuid);
-    void findEntries(QLocalSocket* socket,
+    QJsonArray findMatchingEntries(const QString& dbid,
-                     const QString& nonce,
+                                   const QString& siteUrlStr,
-                     const QString& publicKey,
+                                   const QString& formUrlStr,
-                     const QString& secretKey,
+                                   const QString& realm,
-                     const QString& dbid,
+                                   const StringPairList& keyList,
-                     const QString& hash,
+                                   const bool httpAuth = false);
-                     const QString& requestId,
-                     const QString& siteUrl,
-                     const QString& formUrl,
-                     const QString& realm,
-                     const StringPairList& keyList,
-                     const bool httpAuth = false);
     void requestGlobalAutoType(const QString& search);
     static void convertAttributesToCustomData(QSharedPointer<Database> db);
 
@@ -138,29 +129,12 @@ private:
     QList<Entry*> searchEntries(const QSharedPointer<Database>& db, const QString& siteUrl, const QString& formUrl);
     QList<Entry*> searchEntries(const QString& siteUrl, const QString& formUrl, const StringPairList& keyList);
     QList<Entry*> sortEntries(QList<Entry*>& pwEntries, const QString& siteUrl, const QString& formUrl);
-    void confirmEntries(QLocalSocket* socket,
+    QList<Entry*> confirmEntries(QList<Entry*>& pwEntriesToConfirm,
-                        const QString& incrementedNonce,
+                                 const QString& siteUrl,
-                        const QString& publicKey,
+                                 const QString& siteHost,
-                        const QString& secretKey,
+                                 const QString& formUrl,
-                        const QString& id,
+                                 const QString& realm,
-                        const QString& hash,
+                                 const bool httpAuth);
-                        const QString& requestId,
-                        QList<Entry*>& allowedEntries,
-                        QList<Entry*>& entriesToConfirm,
-                        const QString& siteUrl,
-                        const QString& siteHost,
-                        const QString& formUrl,
-                        const QString& realm,
-                        const bool httpAuth);
-    void sendCredentialsToClient(QList<Entry*>& allowedEntries,
-                                 QLocalSocket* socket,
-                                 const QString& incrementedNonce,
-                                 const QString& publicKey,
-                                 const QString& secretKey,
-                                 const QString& hash,
-                                 const QString& id,
-                                 const QString siteUrl,
-                                 const QString& formUrl);
     QJsonObject prepareEntry(const Entry* entry);
     void allowEntry(Entry* entry, const QString& siteHost, const QString& formUrl, const QString& realm);
     void denyEntry(Entry* entry, const QString& siteHost, const QString& formUrl, const QString& realm);
@@ -196,15 +170,14 @@ private:
     QPointer<BrowserHost> m_browserHost;
     QHash<QString, QSharedPointer<BrowserAction>> m_browserClients;
 
+    bool m_dialogActive;
     bool m_bringToFrontRequested;
     bool m_passwordGeneratorRequested;
-    bool m_accessConfirmRequested;
     WindowState m_prevWindowState;
     QUuid m_keepassBrowserUUID;
 
     QPointer<DatabaseWidget> m_currentDatabaseWidget;
     QScopedPointer<PasswordGeneratorWidget> m_passwordGenerator;
-    QScopedPointer<BrowserAccessControlDialog> m_accessControlDialog;
 
     Q_DISABLE_COPY(BrowserService);