From eee25a1c35259ad4725162d819f79f89d128bb12 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Thu, 10 Aug 2023 08:21:08 -0400 Subject: [PATCH] Fix several issues with Quick Unlock (#9697) * 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. Also improve documentation describing this behavior. * Disable quick unlock in gui tests --- docs/topics/DatabaseOperations.adoc | 2 +- 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 ++ 9 files changed, 51 insertions(+), 15 deletions(-) diff --git a/docs/topics/DatabaseOperations.adoc b/docs/topics/DatabaseOperations.adoc index 4a5d57f2f..35bcdbb16 100644 --- a/docs/topics/DatabaseOperations.adoc +++ b/docs/topics/DatabaseOperations.adoc @@ -61,7 +61,7 @@ image::database_view.png[] === Quick Unlock On Windows and macOS, subject to hardware availability, your credentials can be securely stored to enable subsequent unlocking of your database through biometric authentication. This is enabled by default on Windows using _Windows Hello_ and on macOS using _Touch ID or Apple Watch_ services. You can disable this feature in the Application Settings under the Security section. -NOTE: On Windows you will be prompted to authenticate to Windows Hello on the initial database unlock. This is required to access the hardware certificate store that encrypts your credentials. +NOTE: On Windows, you will be prompted to authenticate to Windows Hello after unlocking your database with full credentials. This is required to setup Quick Unlock. If you cancel this prompt then Quick Unlock will not be enabled and your database will continue to unlock. .Windows Hello example image::quick_unlock_windows_hello.png[] diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 4b17fe01c..82963af44 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 9c97c8618..5d71e7938 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 acf905044..c4a52e2a0 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 5fcee76a9..b1a59b59a 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 f07e4b1af..319fdf89f 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 cf7579efe..32b8d54c3 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/src/gui/DatabaseOpenWidget.h @@ -45,9 +45,13 @@ public: 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 @@ protected: 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 bdbfda8c7..68698b5fb 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 d639d12f0..be5f44e4e 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