diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 0f706445..2b83e1d2 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -78,10 +78,6 @@ Command Line - - Your decision for above entries will be remembered for the duration the requesting client is running. - - Details @@ -95,7 +91,15 @@ - Deny All + Your decision will be remembered for the duration while both the requesting client AND KeePassXC are running. + + + + Deny All && Future + + + + Allow All && &Future @@ -5398,6 +5402,14 @@ We recommend you use the AppImage available on our downloads page. Disconnect this application + + Reset + + + + Reset any remembered decisions for this application + + Merger diff --git a/src/fdosecrets/dbus/DBusClient.cpp b/src/fdosecrets/dbus/DBusClient.cpp index 6e0f6348..36cb927b 100644 --- a/src/fdosecrets/dbus/DBusClient.cpp +++ b/src/fdosecrets/dbus/DBusClient.cpp @@ -73,8 +73,8 @@ namespace FdoSecrets bool DBusClient::itemKnown(const QUuid& uuid) const { - return m_authorizedAll || m_allowed.contains(uuid) || m_allowedOnce.contains(uuid) || m_denied.contains(uuid) - || m_deniedOnce.contains(uuid); + return m_authorizedAll != AuthDecision::Undecided || m_allowed.contains(uuid) || m_allowedOnce.contains(uuid) + || m_denied.contains(uuid) || m_deniedOnce.contains(uuid); } bool DBusClient::itemAuthorized(const QUuid& uuid) const @@ -83,10 +83,16 @@ namespace FdoSecrets // everyone is authorized if this is not enabled return true; } - if (m_authorizedAll) { - // this client is trusted + + // check if we have catch-all decision + if (m_authorizedAll == AuthDecision::Allowed) { return true; } + if (m_authorizedAll == AuthDecision::Denied) { + return false; + } + + // individual decisions if (m_deniedOnce.contains(uuid) || m_denied.contains(uuid)) { // explicitly denied return false; @@ -132,14 +138,21 @@ namespace FdoSecrets } } - void DBusClient::setAllAuthorized(bool authorized) + void DBusClient::setAllAuthorized(AuthDecision authorized) { + // once variants doesn't make sense here + if (authorized == AuthDecision::AllowedOnce) { + authorized = AuthDecision::Allowed; + } + if (authorized == AuthDecision::DeniedOnce) { + authorized = AuthDecision::Denied; + } m_authorizedAll = authorized; } void DBusClient::clearAuthorization() { - m_authorizedAll = false; + m_authorizedAll = AuthDecision::Undecided; m_allowed.clear(); m_allowedOnce.clear(); m_denied.clear(); diff --git a/src/fdosecrets/dbus/DBusClient.h b/src/fdosecrets/dbus/DBusClient.h index 4fc333e1..3d9e94d0 100644 --- a/src/fdosecrets/dbus/DBusClient.h +++ b/src/fdosecrets/dbus/DBusClient.h @@ -159,7 +159,7 @@ namespace FdoSecrets /** * Authorize client to access all items. */ - void setAllAuthorized(bool authorized = true); + void setAllAuthorized(AuthDecision authorized); /** * Forget all previous authorization. @@ -176,7 +176,7 @@ namespace FdoSecrets QPointer m_dbus; PeerInfo m_process; - bool m_authorizedAll{false}; + AuthDecision m_authorizedAll{AuthDecision::Undecided}; QSet m_allowed{}; QSet m_denied{}; diff --git a/src/fdosecrets/objects/Prompt.cpp b/src/fdosecrets/objects/Prompt.cpp index 58fdfa4e..aa0de540 100644 --- a/src/fdosecrets/objects/Prompt.cpp +++ b/src/fdosecrets/objects/Prompt.cpp @@ -303,7 +303,7 @@ namespace FdoSecrets } continue; } - // attach a temporary property so later we can get the item + // attach a temporary property, so later we can get the item // back from the dialog's result entry->setProperty(FdoSecretsBackend, QVariant::fromValue(item.data())); entries << entry; @@ -317,11 +317,11 @@ namespace FdoSecrets connect(ac, &AccessControlDialog::finished, ac, &AccessControlDialog::deleteLater); ac->open(); } else { - itemUnlockFinished({}); + itemUnlockFinished({}, AuthDecision::Undecided); } } - void UnlockPrompt::itemUnlockFinished(const QHash& decisions) + void UnlockPrompt::itemUnlockFinished(const QHash& decisions, AuthDecision forFutureEntries) { auto client = m_client.lock(); if (!client) { @@ -345,6 +345,9 @@ namespace FdoSecrets m_numRejected += 1; } } + if (forFutureEntries != AuthDecision::Undecided) { + client->setAllAuthorized(forFutureEntries); + } // if anything is not unlocked, treat the whole prompt as dismissed // so the client has a chance to handle the error finishPrompt(m_numRejected > 0); diff --git a/src/fdosecrets/objects/Prompt.h b/src/fdosecrets/objects/Prompt.h index 2e9ffed9..59934356 100644 --- a/src/fdosecrets/objects/Prompt.h +++ b/src/fdosecrets/objects/Prompt.h @@ -169,7 +169,7 @@ namespace FdoSecrets QVariant currentResult() const override; void collectionUnlockFinished(bool accepted); - void itemUnlockFinished(const QHash& results); + void itemUnlockFinished(const QHash& results, AuthDecision forFutureEntries); void unlockItems(); static constexpr auto FdoSecretsBackend = "FdoSecretsBackend"; diff --git a/src/fdosecrets/widgets/AccessControlDialog.cpp b/src/fdosecrets/widgets/AccessControlDialog.cpp index 2c8fd696..8e3a3e1d 100644 --- a/src/fdosecrets/widgets/AccessControlDialog.cpp +++ b/src/fdosecrets/widgets/AccessControlDialog.cpp @@ -68,8 +68,6 @@ AccessControlDialog::AccessControlDialog(QWindow* parent, m_ui->itemsTable->resizeColumnsToContents(); // the info widget - m_ui->rememberMsg->setMessageType(MessageWidget::Information); - m_ui->rememberMsg->show(); // sync with m_rememberCheck->setChecked(true) m_ui->exePathWarn->setMessageType(MessageWidget::Warning); m_ui->exePathWarn->hide(); @@ -81,19 +79,28 @@ AccessControlDialog::AccessControlDialog(QWindow* parent, m_ui->buttonBox->addButton(detailsButtonText + QStringLiteral(" >>"), QDialogButtonBox::HelpRole); detailsButton->setCheckable(true); + QString tooltip = QStringLiteral("

%1

") + .arg(tr("Your decision will be remembered for the duration while both the requesting client " + "AND KeePassXC are running.")); + m_rememberCheck = new QCheckBox(tr("Remember"), this); m_rememberCheck->setObjectName("rememberCheck"); // for testing m_rememberCheck->setChecked(true); m_ui->buttonBox->addButton(m_rememberCheck, QDialogButtonBox::ActionRole); + m_rememberCheck->setToolTip(tooltip); auto allowButton = m_ui->buttonBox->addButton(tr("Allow Selected"), QDialogButtonBox::AcceptRole); allowButton->setDefault(true); - auto cancelButton = m_ui->buttonBox->addButton(tr("Deny All"), QDialogButtonBox::RejectRole); + auto cancelButton = m_ui->buttonBox->addButton(tr("Deny All && Future"), QDialogButtonBox::RejectRole); + cancelButton->setToolTip(tooltip); + + auto allowAllButton = m_ui->buttonBox->addButton(tr("Allow All && &Future"), QDialogButtonBox::AcceptRole); + allowAllButton->setToolTip(tooltip); connect(cancelButton, &QPushButton::clicked, this, [this]() { done(DenyAll); }); connect(allowButton, &QPushButton::clicked, this, [this]() { done(AllowSelected); }); - connect(m_rememberCheck, &QCheckBox::clicked, this, &AccessControlDialog::rememberChecked); + connect(allowAllButton, &QPushButton::clicked, this, [this]() { done(AllowAll); }); connect(detailsButton, &QPushButton::clicked, this, [=](bool checked) { m_ui->detailsContainer->setVisible(checked); if (checked) { @@ -119,6 +126,9 @@ AccessControlDialog::AccessControlDialog(QWindow* parent, detailsButton->click(); } + // adjust size after we initialize the button box + adjustSize(); + allowButton->setFocus(); } @@ -146,15 +156,6 @@ void AccessControlDialog::setupDetails(const FdoSecrets::PeerInfo& info) m_ui->detailsContainer->hide(); } -void AccessControlDialog::rememberChecked(bool checked) -{ - if (checked) { - m_ui->rememberMsg->animatedShow(); - } else { - m_ui->rememberMsg->animatedHide(); - } -} - void AccessControlDialog::denyEntryClicked(Entry* entry, const QModelIndex& index) { m_decisions.insert(entry, AuthDecision::Denied); @@ -166,32 +167,35 @@ void AccessControlDialog::denyEntryClicked(Entry* entry, const QModelIndex& inde void AccessControlDialog::dialogFinished(int result) { - auto allow = m_rememberCheck->isChecked() ? AuthDecision::Allowed : AuthDecision::AllowedOnce; - auto deny = m_rememberCheck->isChecked() ? AuthDecision::Denied : AuthDecision::DeniedOnce; + auto decision = AuthDecision::Undecided; + auto futureDecision = AuthDecision::Undecided; + switch (result) { + case AllowSelected: + decision = m_rememberCheck->isChecked() ? AuthDecision::Allowed : AuthDecision::AllowedOnce; + break; + case AllowAll: + decision = AuthDecision::Allowed; + futureDecision = AuthDecision::Allowed; + break; + case DenyAll: + decision = AuthDecision::Denied; + futureDecision = AuthDecision::Denied; + break; + case Rejected: + default: + break; + } for (int row = 0; row != m_model->rowCount({}); ++row) { auto entry = m_model->data(m_model->index(row, 2), Qt::EditRole).value(); auto selected = m_model->data(m_model->index(row, 0), Qt::CheckStateRole).value(); Q_ASSERT(entry); - switch (result) { - case AllowSelected: - if (selected) { - m_decisions.insert(entry, allow); - } else { - m_decisions.insert(entry, AuthDecision::Undecided); - } - break; - case DenyAll: - m_decisions.insert(entry, deny); - break; - case Rejected: - default: - m_decisions.insert(entry, AuthDecision::Undecided); - break; - } + + auto undecided = result == AllowSelected && !selected; + m_decisions.insert(entry, undecided ? AuthDecision::Undecided : decision); } - emit finished(m_decisions); + emit finished(m_decisions, futureDecision); } QHash AccessControlDialog::decisions() const diff --git a/src/fdosecrets/widgets/AccessControlDialog.h b/src/fdosecrets/widgets/AccessControlDialog.h index 7f491f7c..37008500 100644 --- a/src/fdosecrets/widgets/AccessControlDialog.h +++ b/src/fdosecrets/widgets/AccessControlDialog.h @@ -66,16 +66,16 @@ public: { Rejected, AllowSelected, + AllowAll, DenyAll, }; QHash decisions() const; signals: - void finished(const QHash& results); + void finished(const QHash& results, AuthDecision forFutureEntries); private slots: - void rememberChecked(bool checked); void denyEntryClicked(Entry* entry, const QModelIndex& index); void dialogFinished(int result); diff --git a/src/fdosecrets/widgets/AccessControlDialog.ui b/src/fdosecrets/widgets/AccessControlDialog.ui index 3e478527..faa35ed3 100644 --- a/src/fdosecrets/widgets/AccessControlDialog.ui +++ b/src/fdosecrets/widgets/AccessControlDialog.ui @@ -122,16 +122,6 @@ - - - - Your decision for above entries will be remembered for the duration the requesting client is running. - - - false - - - diff --git a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp index 842a40b9..692a7561 100644 --- a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp +++ b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp @@ -26,7 +26,8 @@ #include "gui/DatabaseWidget.h" -#include +#include +#include using FdoSecrets::DBusClientPtr; using FdoSecrets::SettingsClientModel; @@ -34,7 +35,7 @@ using FdoSecrets::SettingsDatabaseModel; namespace { - class ManageDatabase : public QToolBar + class ManageDatabase : public QWidget { Q_OBJECT @@ -42,18 +43,9 @@ namespace public: explicit ManageDatabase(FdoSecretsPlugin* plugin, QWidget* parent = nullptr) - : QToolBar(parent) + : QWidget(parent) , m_plugin(plugin) { - setFloatable(false); - setMovable(false); - - // use a dummy widget to center the buttons - auto spacer = new QWidget(this); - spacer->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Preferred); - spacer->setVisible(true); - addWidget(spacer); - // db settings m_dbSettingsAct = new QAction(tr("Database settings"), this); m_dbSettingsAct->setIcon(icons()->icon(QStringLiteral("document-edit"))); @@ -63,14 +55,12 @@ namespace if (!m_dbWidget) { return; } - auto db = m_dbWidget; m_plugin->serviceInstance()->doSwitchToDatabaseSettings(m_dbWidget); }); - addAction(m_dbSettingsAct); // unlock/lock m_lockAct = new QAction(tr("Unlock database"), this); - m_lockAct->setIcon(icons()->icon(QStringLiteral("object-locked"))); + m_lockAct->setIcon(icons()->icon(QStringLiteral("object-unlocked"))); m_lockAct->setToolTip(tr("Unlock database to show more information")); connect(m_lockAct, &QAction::triggered, this, [this]() { if (!m_dbWidget) { @@ -83,13 +73,23 @@ namespace } }); - addAction(m_lockAct); + // layout + auto dbSettingsBtn = new QToolButton(this); + dbSettingsBtn->setAutoRaise(true); + dbSettingsBtn->setDefaultAction(m_dbSettingsAct); - // use a dummy widget to center the buttons - spacer = new QWidget(this); - spacer->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Preferred); - spacer->setVisible(true); - addWidget(spacer); + auto lockBtn = new QToolButton(this); + lockBtn->setAutoRaise(true); + lockBtn->setDefaultAction(m_lockAct); + + auto layout = new QHBoxLayout(this); + layout->setContentsMargins(1, 1, 1, 1); + layout->setSpacing(1); + + layout->addStretch(); + layout->addWidget(dbSettingsBtn); + layout->addWidget(lockBtn); + layout->addStretch(); } DatabaseWidget* dbWidget() const @@ -127,14 +127,20 @@ namespace return; } connect(m_dbWidget, &DatabaseWidget::databaseLocked, this, [this]() { + if (!m_lockAct || !m_dbSettingsAct) { + return; + } m_lockAct->setText(tr("Unlock database")); - m_lockAct->setIcon(icons()->icon(QStringLiteral("object-locked"))); + m_lockAct->setIcon(icons()->icon(QStringLiteral("object-unlocked"))); m_lockAct->setToolTip(tr("Unlock database to show more information")); m_dbSettingsAct->setEnabled(false); }); connect(m_dbWidget, &DatabaseWidget::databaseUnlocked, this, [this]() { + if (!m_lockAct || !m_dbSettingsAct) { + return; + } m_lockAct->setText(tr("Lock database")); - m_lockAct->setIcon(icons()->icon(QStringLiteral("object-unlocked"))); + m_lockAct->setIcon(icons()->icon(QStringLiteral("object-locked"))); m_lockAct->setToolTip(tr("Lock database")); m_dbSettingsAct->setEnabled(true); }); @@ -147,7 +153,7 @@ namespace QAction* m_lockAct = nullptr; }; - class ManageSession : public QToolBar + class ManageSession : public QWidget { Q_OBJECT @@ -155,17 +161,8 @@ namespace public: explicit ManageSession(QWidget* parent = nullptr) - : QToolBar(parent) + : QWidget(parent) { - setFloatable(false); - setMovable(false); - - // use a dummy widget to center the buttons - auto spacer = new QWidget(this); - spacer->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Preferred); - spacer->setVisible(true); - addWidget(spacer); - auto disconnectAct = new QAction(tr("Disconnect"), this); disconnectAct->setIcon(icons()->icon(QStringLiteral("dialog-close"))); disconnectAct->setToolTip(tr("Disconnect this application")); @@ -174,13 +171,33 @@ namespace m_client->disconnectDBus(); } }); - addAction(disconnectAct); - // use a dummy widget to center the buttons - spacer = new QWidget(this); - spacer->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Preferred); - spacer->setVisible(true); - addWidget(spacer); + auto resetAct = new QAction(tr("Reset"), this); + resetAct->setIcon(icons()->icon(QStringLiteral("refresh"))); + resetAct->setToolTip(tr("Reset any remembered decisions for this application")); + connect(resetAct, &QAction::triggered, this, [this]() { + if (m_client) { + m_client->clearAuthorization(); + } + }); + + // layout + auto disconnectBtn = new QToolButton(this); + disconnectBtn->setAutoRaise(true); + disconnectBtn->setDefaultAction(disconnectAct); + + auto resetBtn = new QToolButton(this); + resetBtn->setAutoRaise(true); + resetBtn->setDefaultAction(resetAct); + + auto layout = new QHBoxLayout(this); + layout->setContentsMargins(1, 1, 1, 1); + layout->setSpacing(1); + + layout->addStretch(); + layout->addWidget(resetBtn); + layout->addWidget(disconnectBtn); + layout->addStretch(); } const DBusClientPtr& client() const diff --git a/tests/gui/TestGuiFdoSecrets.cpp b/tests/gui/TestGuiFdoSecrets.cpp index 5e2a52e2..a2b5647d 100644 --- a/tests/gui/TestGuiFdoSecrets.cpp +++ b/tests/gui/TestGuiFdoSecrets.cpp @@ -608,6 +608,62 @@ void TestGuiFdoSecrets::testServiceUnlockItems() DBUS_COMPARE(item->locked(), false); } +void TestGuiFdoSecrets::testServiceUnlockItemsIncludeFutureEntries() +{ + FdoSecrets::settings()->setConfirmAccessItem(true); + + auto service = enableService(); + VERIFY(service); + auto coll = getDefaultCollection(service); + VERIFY(coll); + auto item = getFirstItem(coll); + VERIFY(item); + auto sess = openSession(service, DhIetf1024Sha256Aes128CbcPkcs7::Algorithm); + VERIFY(sess); + + DBUS_COMPARE(item->locked(), true); + + { + DBUS_GET2(unlocked, promptPath, service->Unlock({QDBusObjectPath(item->path())})); + // nothing is unlocked immediately without user's action + COMPARE(unlocked, {}); + + auto prompt = getProxy(promptPath); + VERIFY(prompt); + QSignalSpy spyPromptCompleted(prompt.data(), SIGNAL(Completed(bool, QDBusVariant))); + VERIFY(spyPromptCompleted.isValid()); + + // nothing is unlocked yet + COMPARE(spyPromptCompleted.count(), 0); + DBUS_COMPARE(item->locked(), true); + + // drive the prompt + DBUS_VERIFY(prompt->Prompt("")); + // remember and include future entries + VERIFY(driveAccessControlDialog(true, true)); + + 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); + } + + // check other entries are also unlocked + { + DBUS_GET(itemPaths, coll->items()); + VERIFY(itemPaths.size() > 1); + auto anotherItem = getProxy(itemPaths.last()); + VERIFY(anotherItem); + DBUS_COMPARE(anotherItem->locked(), false); + } +} + void TestGuiFdoSecrets::testServiceLock() { auto service = enableService(); @@ -1635,7 +1691,7 @@ TestGuiFdoSecrets::encryptPassword(QByteArray value, QString contentType, const return m_clientCipher->encrypt(ss.unmarshal(m_plugin->dbus())).marshal(); } -bool TestGuiFdoSecrets::driveAccessControlDialog(bool remember) +bool TestGuiFdoSecrets::driveAccessControlDialog(bool remember, bool includeFutureEntries) { processEvents(); for (auto w : QApplication::topLevelWidgets()) { @@ -1647,7 +1703,13 @@ bool TestGuiFdoSecrets::driveAccessControlDialog(bool remember) auto rememberCheck = dlg->findChild("rememberCheck"); VERIFY(rememberCheck); rememberCheck->setChecked(remember); - dlg->done(AccessControlDialog::AllowSelected); + + if (includeFutureEntries) { + dlg->done(AccessControlDialog::AllowAll); + } else { + dlg->done(AccessControlDialog::AllowSelected); + } + processEvents(); VERIFY(dlg->isHidden()); return true; diff --git a/tests/gui/TestGuiFdoSecrets.h b/tests/gui/TestGuiFdoSecrets.h index 285619f8..03f84adc 100644 --- a/tests/gui/TestGuiFdoSecrets.h +++ b/tests/gui/TestGuiFdoSecrets.h @@ -71,6 +71,7 @@ private slots: void testServiceUnlock(); void testServiceUnlockDatabaseConcurrent(); void testServiceUnlockItems(); + void testServiceUnlockItemsIncludeFutureEntries(); void testServiceLock(); void testServiceLockConcurrent(); @@ -103,7 +104,7 @@ private slots: private: bool driveUnlockDialog(); bool driveNewDatabaseWizard(); - bool driveAccessControlDialog(bool remember = true); + bool driveAccessControlDialog(bool remember = true, bool includeFutureEntries = false); bool waitForSignal(QSignalSpy& spy, int expectedCount); void processEvents();