From fb848775abd64ff7a8a4db5aec2bf9dc8e585328 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sat, 5 Aug 2023 22:22:37 -0400 Subject: [PATCH] Fix several issues with Quick Unlock * Fix #7892 - Pressing escape when the quick unlock prompt is shown will now go back to the main unlock dialog view. * Fix #9030 - Quick unlock will be automatically invoked in the unlock dialog upon being shown. * Fix #9554 - Quick unlock application setting will be updated every time the settings widget is shown instead of just on first launch. * Show warning that quick unlock is not enabled if user cancels Windows Hello prompt. This should limit people thinking there is a security issue. * Disable quick unlock in gui tests --- share/translations/keepassxc_en.ts | 4 ++++ src/gui/ApplicationSettingsWidget.cpp | 21 +++++++++++---------- src/gui/DatabaseOpenDialog.cpp | 10 ++++++++++ src/gui/DatabaseOpenDialog.h | 3 +++ src/gui/DatabaseOpenWidget.cpp | 14 +++++++++++++- src/gui/DatabaseOpenWidget.h | 8 +++++--- tests/gui/TestGuiBrowser.cpp | 2 ++ tests/gui/TestGuiFdoSecrets.cpp | 2 ++ 8 files changed, 50 insertions(+), 14 deletions(-) diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 865a42344f..5fb599cc0a 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -1579,6 +1579,10 @@ If you do not have a key file, please leave the field empty. Failed to authenticate with Windows Hello: %1 + + Windows Hello setup was canceled or failed. Quick unlock has not been enabled. + + DatabaseSettingWidgetMetaData diff --git a/src/gui/ApplicationSettingsWidget.cpp b/src/gui/ApplicationSettingsWidget.cpp index 9c97c8618b..5d71e79381 100644 --- a/src/gui/ApplicationSettingsWidget.cpp +++ b/src/gui/ApplicationSettingsWidget.cpp @@ -164,15 +164,6 @@ ApplicationSettingsWidget::ApplicationSettingsWidget(QWidget* parent) m_generalUi->faviconTimeoutLabel->setVisible(false); m_generalUi->faviconTimeoutSpinBox->setVisible(false); #endif - - bool showQuickUnlock = false; -#if defined(Q_OS_MACOS) - showQuickUnlock = TouchID::getInstance().isAvailable(); -#elif defined(Q_CC_MSVC) - showQuickUnlock = getWindowsHello()->isAvailable(); - connect(getWindowsHello(), &WindowsHello::availableChanged, m_secUi->quickUnlockCheckBox, &QCheckBox::setVisible); -#endif - m_secUi->quickUnlockCheckBox->setVisible(showQuickUnlock); } ApplicationSettingsWidget::~ApplicationSettingsWidget() = default; @@ -323,6 +314,14 @@ void ApplicationSettingsWidget::loadSettings() m_secUi->EnableCopyOnDoubleClickCheckBox->setChecked( config()->get(Config::Security_EnableCopyOnDoubleClick).toBool()); + bool quickUnlockAvailable = false; +#if defined(Q_OS_MACOS) + quickUnlockAvailable = TouchID::getInstance().isAvailable(); +#elif defined(Q_CC_MSVC) + quickUnlockAvailable = getWindowsHello()->isAvailable(); + connect(getWindowsHello(), &WindowsHello::availableChanged, m_secUi->quickUnlockCheckBox, &QCheckBox::setEnabled); +#endif + m_secUi->quickUnlockCheckBox->setEnabled(quickUnlockAvailable); m_secUi->quickUnlockCheckBox->setChecked(config()->get(Config::Security_QuickUnlock).toBool()); for (const ExtraPage& page : asConst(m_extraPages)) { @@ -435,7 +434,9 @@ void ApplicationSettingsWidget::saveSettings() m_secUi->NoConfirmMoveEntryToRecycleBinCheckBox->isChecked()); config()->set(Config::Security_EnableCopyOnDoubleClick, m_secUi->EnableCopyOnDoubleClickCheckBox->isChecked()); - config()->set(Config::Security_QuickUnlock, m_secUi->quickUnlockCheckBox->isChecked()); + if (m_secUi->quickUnlockCheckBox->isEnabled()) { + config()->set(Config::Security_QuickUnlock, m_secUi->quickUnlockCheckBox->isChecked()); + } // Security: clear storage if related settings are disabled if (!config()->get(Config::RememberLastDatabases).toBool()) { diff --git a/src/gui/DatabaseOpenDialog.cpp b/src/gui/DatabaseOpenDialog.cpp index acf9050445..c4a52e2a0c 100644 --- a/src/gui/DatabaseOpenDialog.cpp +++ b/src/gui/DatabaseOpenDialog.cpp @@ -80,6 +80,16 @@ DatabaseOpenDialog::DatabaseOpenDialog(QWidget* parent) connect(shortcut, &QShortcut::activated, this, [this]() { selectTabOffset(1); }); } +void DatabaseOpenDialog::showEvent(QShowEvent* event) +{ + QDialog::showEvent(event); + QTimer::singleShot(100, this, [=] { + if (m_view->isOnQuickUnlockScreen() && !m_view->unlockingDatabase()) { + m_view->triggerQuickUnlock(); + } + }); +} + void DatabaseOpenDialog::selectTabOffset(int offset) { if (offset == 0 || m_tabBar->count() <= 1) { diff --git a/src/gui/DatabaseOpenDialog.h b/src/gui/DatabaseOpenDialog.h index 5fcee76a9b..b1a59b59a7 100644 --- a/src/gui/DatabaseOpenDialog.h +++ b/src/gui/DatabaseOpenDialog.h @@ -58,6 +58,9 @@ public slots: void complete(bool accepted); void tabChanged(int index); +protected: + void showEvent(QShowEvent* event) override; + private: void selectTabOffset(int offset); diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index f07e4b1afa..319fdf89ff 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -139,6 +139,7 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) // QuickUnlock actions connect(m_ui->quickUnlockButton, &QPushButton::pressed, this, [this] { openDatabase(); }); connect(m_ui->resetQuickUnlockButton, &QPushButton::pressed, this, [this] { resetQuickUnlock(); }); + m_ui->resetQuickUnlockButton->setShortcut(Qt::Key_Escape); } DatabaseOpenWidget::~DatabaseOpenWidget() = default; @@ -284,7 +285,11 @@ void DatabaseOpenWidget::openDatabase() auto keyData = databaseKey->serialize(); #if defined(Q_CC_MSVC) // Store the password using Windows Hello - getWindowsHello()->storeKey(m_filename, keyData); + if (!getWindowsHello()->storeKey(m_filename, keyData)) { + getMainWindow()->displayTabMessage( + tr("Windows Hello setup was canceled or failed. Quick unlock has not been enabled."), + MessageWidget::MessageType::Warning); + } #elif defined(Q_OS_MACOS) // Store the password using TouchID TouchID::getInstance().storeKey(m_filename, keyData); @@ -534,6 +539,13 @@ bool DatabaseOpenWidget::isOnQuickUnlockScreen() return m_ui->centralStack->currentIndex() == 1; } +void DatabaseOpenWidget::triggerQuickUnlock() +{ + if (isOnQuickUnlockScreen()) { + m_ui->quickUnlockButton->click(); + } +} + /** * Reset installed quick unlock secrets. * diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h index cf7579efe7..32b8d54c37 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/src/gui/DatabaseOpenWidget.h @@ -45,9 +45,13 @@ class DatabaseOpenWidget : public DialogyWidget void clearForms(); void enterKey(const QString& pw, const QString& keyFile); QSharedPointer database(); - void resetQuickUnlock(); bool unlockingDatabase(); + // Quick Unlock helper functions + bool isOnQuickUnlockScreen(); + void triggerQuickUnlock(); + void resetQuickUnlock(); + signals: void dialogFinished(bool accepted); @@ -56,8 +60,6 @@ class DatabaseOpenWidget : public DialogyWidget void hideEvent(QHideEvent* event) override; QSharedPointer buildDatabaseKey(); void setUserInteractionLock(bool state); - // Quick Unlock helper functions - bool isOnQuickUnlockScreen(); const QScopedPointer m_ui; QSharedPointer m_db; diff --git a/tests/gui/TestGuiBrowser.cpp b/tests/gui/TestGuiBrowser.cpp index bdbfda8c7a..68698b5fb8 100644 --- a/tests/gui/TestGuiBrowser.cpp +++ b/tests/gui/TestGuiBrowser.cpp @@ -68,6 +68,8 @@ void TestGuiBrowser::initTestCase() config()->set(Config::GUI_ShowTrayIcon, true); // Disable the update check first time alert config()->set(Config::UpdateCheckMessageShown, true); + // Disable quick unlock + config()->set(Config::Security_QuickUnlock, false); m_mainWindow.reset(new MainWindow()); m_tabWidget = m_mainWindow->findChild("tabWidget"); diff --git a/tests/gui/TestGuiFdoSecrets.cpp b/tests/gui/TestGuiFdoSecrets.cpp index d639d12f0a..be5f44e4e3 100644 --- a/tests/gui/TestGuiFdoSecrets.cpp +++ b/tests/gui/TestGuiFdoSecrets.cpp @@ -142,6 +142,8 @@ void TestGuiFdoSecrets::initTestCase() config()->set(Config::AutoSaveOnExit, false); config()->set(Config::GUI_ShowTrayIcon, true); config()->set(Config::UpdateCheckMessageShown, true); + // Disable quick unlock + config()->set(Config::Security_QuickUnlock, false); // Disable secret service integration (activate within individual tests to test the plugin) FdoSecrets::settings()->setEnabled(false); // activate within individual tests