Improvements to confirm access dialog

* Disable access to entries immediately within the dialog
* Use checkboxes instead of row selection
* Add button to deny all access immediately

src/browser/BrowserAccessControlDialog.cpp

@@ -25,29 +25,54 @@ BrowserAccessControlDialog::BrowserAccessControlDialog(QWidget* parent)
     : QDialog(parent)
     , m_ui(new Ui::BrowserAccessControlDialog())
 {
-    this->setWindowFlags(windowFlags() | Qt::WindowStaysOnTopHint);
+    setWindowFlags(windowFlags() | Qt::WindowStaysOnTopHint);
 
     m_ui->setupUi(this);
-    connect(m_ui->allowButton, SIGNAL(clicked()), this, SLOT(accept()));
-    connect(m_ui->denyButton, SIGNAL(clicked()), this, SLOT(reject()));
+
+    connect(m_ui->allowButton, SIGNAL(clicked()), SLOT(accept()));
+    connect(m_ui->cancelButton, SIGNAL(clicked()), SLOT(reject()));
 }
 
 BrowserAccessControlDialog::~BrowserAccessControlDialog()
 {
 }
 
-void BrowserAccessControlDialog::setUrl(const QString& url)
+void BrowserAccessControlDialog::setItems(const QList<Entry*>& items, const QString& hostname, bool httpAuth)
 {
-    m_ui->label->setText(QString(tr("%1 has requested access to passwords for the following item(s).\n"
-                                    "Please select whether you want to allow access."))
-                             .arg(QUrl(url).host()));
-}
+    m_ui->siteLabel->setText(m_ui->siteLabel->text().arg(hostname));
 
-void BrowserAccessControlDialog::setItems(const QList<Entry*>& items)
-{
-    for (Entry* entry : items) {
-        m_ui->itemsList->addItem(entry->title() + " - " + entry->username());
+    m_ui->rememberDecisionCheckBox->setVisible(!httpAuth);
+    m_ui->rememberDecisionCheckBox->setChecked(false);
+
+    m_ui->itemsTable->setRowCount(items.count());
+    m_ui->itemsTable->setColumnCount(2);
+
+    int row = 0;
+    for (const auto& entry : items) {
+        auto item = new QTableWidgetItem();
+        item->setText(entry->title() + " - " + entry->username());
+        item->setData(Qt::UserRole, row);
+        item->setCheckState(Qt::Checked);
+        item->setFlags(item->flags() | Qt::ItemIsUserCheckable);
+        m_ui->itemsTable->setItem(row, 0, item);
+
+        auto disableButton = new QPushButton(tr("Disable for this site"));
+        connect(disableButton, &QAbstractButton::pressed, [&, item] {
+            emit disableAccess(item);
+            m_ui->itemsTable->removeRow(item->row());
+            if (m_ui->itemsTable->rowCount() == 0) {
+                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();
 }
 
 bool BrowserAccessControlDialog::remember() const
@@ -55,12 +80,26 @@ bool BrowserAccessControlDialog::remember() const
     return m_ui->rememberDecisionCheckBox->isChecked();
 }
 
-void BrowserAccessControlDialog::setRemember(bool r)
+QList<QTableWidgetItem*> BrowserAccessControlDialog::getSelectedEntries() const
 {
-    m_ui->rememberDecisionCheckBox->setChecked(r);
+    QList<QTableWidgetItem*> selected;
+    for (int i = 0; i < m_ui->itemsTable->rowCount(); ++i) {
+        auto item = m_ui->itemsTable->item(i, 0);
+        if (item->checkState() == Qt::Checked) {
+            selected.append(item);
+        }
+    }
+    return selected;
 }
 
-void BrowserAccessControlDialog::setHTTPAuth(bool httpAuth)
+QList<QTableWidgetItem*> BrowserAccessControlDialog::getNonSelectedEntries() const
 {
-    m_ui->rememberDecisionCheckBox->setVisible(!httpAuth);
+    QList<QTableWidgetItem*> notSelected;
+    for (int i = 0; i < m_ui->itemsTable->rowCount(); ++i) {
+        auto item = m_ui->itemsTable->item(i, 0);
+        if (item->checkState() != Qt::Checked) {
+            notSelected.append(item);
+        }
+    }
+    return notSelected;
 }

src/browser/BrowserAccessControlDialog.h

@@ -21,6 +21,7 @@
 
 #include <QDialog>
 #include <QScopedPointer>
+#include <QTableWidgetItem>
 
 class Entry;
 
@@ -35,13 +36,16 @@ class BrowserAccessControlDialog : public QDialog
 
 public:
     explicit BrowserAccessControlDialog(QWidget* parent = nullptr);
-    ~BrowserAccessControlDialog();
+    ~BrowserAccessControlDialog() override;
 
-    void setUrl(const QString& url);
-    void setItems(const QList<Entry*>& items);
+    void setItems(const QList<Entry*>& items, const QString& hostname, bool httpAuth);
     bool remember() const;
-    void setRemember(bool r);
-    void setHTTPAuth(bool httpAuth);
+
+    QList<QTableWidgetItem*> getSelectedEntries() const;
+    QList<QTableWidgetItem*> getNonSelectedEntries() const;
+
+signals:
+    void disableAccess(QTableWidgetItem* item);
 
 private:
     QScopedPointer<Ui::BrowserAccessControlDialog> m_ui;

src/browser/BrowserAccessControlDialog.ui

@@ -6,29 +6,50 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>400</width>
-    <height>221</height>
+    <width>405</width>
+    <height>200</height>
    </rect>
   </property>
   <property name="windowTitle">
-   <string>KeePassXC-Browser Confirm Access</string>
+   <string>KeePassXC - Browser Access Request</string>
   </property>
   <layout class="QVBoxLayout" name="verticalLayout">
    <item>
-    <widget class="QLabel" name="label">
+    <widget class="QLabel" name="siteLabel">
+     <property name="font">
+      <font>
+       <weight>75</weight>
+       <bold>true</bold>
+      </font>
+     </property>
      <property name="text">
-      <string/>
+      <string>%1 is requesting access to the following entries:</string>
+     </property>
+     <property name="alignment">
+      <set>Qt::AlignCenter</set>
      </property>
     </widget>
    </item>
    <item>
-    <widget class="QListWidget" name="itemsList"/>
-   </item>
-   <item>
-    <widget class="QCheckBox" name="rememberDecisionCheckBox">
-     <property name="text">
-      <string>Remember this decision</string>
+    <widget class="QTableWidget" name="itemsTable">
+     <property name="editTriggers">
+      <set>QAbstractItemView::NoEditTriggers</set>
      </property>
+     <property name="showDropIndicator" stdset="0">
+      <bool>false</bool>
+     </property>
+     <property name="selectionMode">
+      <enum>QAbstractItemView::NoSelection</enum>
+     </property>
+     <property name="cornerButtonEnabled">
+      <bool>false</bool>
+     </property>
+     <attribute name="horizontalHeaderVisible">
+      <bool>false</bool>
+     </attribute>
+     <attribute name="verticalHeaderVisible">
+      <bool>false</bool>
+     </attribute>
     </widget>
    </item>
    <item>
@@ -47,22 +68,35 @@
       </spacer>
      </item>
      <item>
-      <widget class="QPushButton" name="allowButton">
+      <widget class="QCheckBox" name="rememberDecisionCheckBox">
+       <property name="toolTip">
+        <string>Remember access to checked entries</string>
+       </property>
        <property name="accessibleName">
-        <string>Allow access</string>
+        <string>Remember access to checked entries</string>
        </property>
        <property name="text">
-        <string>Allow</string>
+        <string>Remember</string>
        </property>
       </widget>
      </item>
      <item>
-      <widget class="QPushButton" name="denyButton">
+      <widget class="QPushButton" name="allowButton">
        <property name="accessibleName">
-        <string>Deny access</string>
+        <string>Allow access to entries</string>
        </property>
        <property name="text">
-        <string>Deny</string>
+        <string>Allow Selected</string>
+       </property>
+      </widget>
+     </item>
+     <item>
+      <widget class="QPushButton" name="cancelButton">
+       <property name="text">
+        <string>Deny All</string>
+       </property>
+       <property name="autoDefault">
+        <bool>false</bool>
        </property>
       </widget>
      </item>

src/browser/BrowserService.cpp

@@ -417,8 +417,9 @@ QJsonArray BrowserService::findMatchingEntries(const QString& id,
     }
 
     // Confirm entries
-    if (confirmEntries(pwEntriesToConfirm, url, host, submitUrl, realm, httpAuth)) {
-        pwEntries.append(pwEntriesToConfirm);
+    QList<Entry*> selectedEntriesToConfirm = confirmEntries(pwEntriesToConfirm, url, host, submitHost, realm, httpAuth);
+    if (!selectedEntriesToConfirm.isEmpty()) {
+        pwEntries.append(selectedEntriesToConfirm);
     }
 
     if (pwEntries.isEmpty()) {
@@ -788,59 +789,66 @@ QList<Entry*> BrowserService::sortEntries(QList<Entry*>& pwEntries, const QStrin
     return results;
 }
 
-bool BrowserService::confirmEntries(QList<Entry*>& pwEntriesToConfirm,
-                                    const QString& url,
-                                    const QString& host,
-                                    const QString& submitUrl,
-                                    const QString& realm,
-                                    const bool httpAuth)
+QList<Entry*> BrowserService::confirmEntries(QList<Entry*>& pwEntriesToConfirm,
+                                             const QString& url,
+                                             const QString& host,
+                                             const QString& submitHost,
+                                             const QString& realm,
+                                             const bool httpAuth)
 {
     if (pwEntriesToConfirm.isEmpty() || m_dialogActive) {
-        return false;
+        return {};
     }
 
     m_dialogActive = true;
     BrowserAccessControlDialog accessControlDialog;
+
     connect(m_dbTabWidget, SIGNAL(databaseLocked(DatabaseWidget*)), &accessControlDialog, SLOT(reject()));
-    accessControlDialog.setUrl(!submitUrl.isEmpty() ? submitUrl : url);
-    accessControlDialog.setItems(pwEntriesToConfirm);
-    accessControlDialog.setHTTPAuth(httpAuth);
+    connect(&accessControlDialog, &BrowserAccessControlDialog::disableAccess, [&](QTableWidgetItem* item) {
+        auto entry = pwEntriesToConfirm[item->row()];
+        BrowserEntryConfig config;
+        config.load(entry);
+        config.deny(host);
+        if (!submitHost.isEmpty() && host != submitHost) {
+            config.deny(submitHost);
+        }
+        if (!realm.isEmpty()) {
+            config.setRealm(realm);
+        }
+        config.save(entry);
+    });
+
+    accessControlDialog.setItems(pwEntriesToConfirm, !submitHost.isEmpty() ? submitHost : url, httpAuth);
 
     raiseWindow();
     accessControlDialog.show();
     accessControlDialog.activateWindow();
     accessControlDialog.raise();
 
-    const QString submitHost = QUrl(submitUrl).host();
-    int res = accessControlDialog.exec();
-    if (accessControlDialog.remember()) {
-        for (auto* entry : pwEntriesToConfirm) {
-            BrowserEntryConfig config;
-            config.load(entry);
-            if (res == QDialog::Accepted) {
+    QList<Entry*> allowedEntries;
+    if (accessControlDialog.exec() == QDialog::Accepted) {
+        const auto selectedEntries = accessControlDialog.getSelectedEntries();
+        for (auto item : accessControlDialog.getSelectedEntries()) {
+            auto entry = pwEntriesToConfirm[item->row()];
+            if (accessControlDialog.remember()) {
+                BrowserEntryConfig config;
+                config.load(entry);
                 config.allow(host);
-                if (!submitHost.isEmpty() && host != submitHost)
-                    config.allow(submitHost);
-            } else if (res == QDialog::Rejected) {
-                config.deny(host);
                 if (!submitHost.isEmpty() && host != submitHost) {
-                    config.deny(submitHost);
+                    config.allow(submitHost);
                 }
+                if (!realm.isEmpty()) {
+                    config.setRealm(realm);
+                }
+                config.save(entry);
             }
-            if (!realm.isEmpty()) {
-                config.setRealm(realm);
-            }
-            config.save(entry);
+            allowedEntries.append(entry);
         }
     }
 
     m_dialogActive = false;
     hideWindow();
-    if (res == QDialog::Accepted) {
-        return true;
-    }
-
-    return false;
+    return allowedEntries;
 }
 
 QJsonObject BrowserService::prepareEntry(const Entry* entry)

src/browser/BrowserService.h

@@ -118,12 +118,12 @@ private:
 
 private:
     QList<Entry*> sortEntries(QList<Entry*>& pwEntries, const QString& host, const QString& submitUrl);
-    bool confirmEntries(QList<Entry*>& pwEntriesToConfirm,
-                        const QString& url,
-                        const QString& host,
-                        const QString& submitUrl,
-                        const QString& realm,
-                        const bool httpAuth);
+    QList<Entry*> confirmEntries(QList<Entry*>& pwEntriesToConfirm,
+                                 const QString& url,
+                                 const QString& host,
+                                 const QString& submitUrl,
+                                 const QString& realm,
+                                 const bool httpAuth);
     QJsonObject prepareEntry(const Entry* entry);
     Access checkAccess(const Entry* entry, const QString& host, const QString& submitHost, const QString& realm);
     Group* getDefaultEntryGroup(const QSharedPointer<Database>& selectedDb = {});