Skip to content

Commit

Permalink
Fix several issues with Quick Unlock (#9697)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
droidmonkey committed Aug 10, 2023
1 parent 5fb26d6 commit eee25a1
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 15 deletions.
2 changes: 1 addition & 1 deletion docs/topics/DatabaseOperations.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
Expand Down
4 changes: 4 additions & 0 deletions share/translations/keepassxc_en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,10 @@ If you do not have a key file, please leave the field empty.</source>
<source>Failed to authenticate with Windows Hello: %1</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Windows Hello setup was canceled or failed. Quick unlock has not been enabled.</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>DatabaseSettingWidgetMetaData</name>
Expand Down
21 changes: 11 additions & 10 deletions src/gui/ApplicationSettingsWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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()) {
Expand Down
10 changes: 10 additions & 0 deletions src/gui/DatabaseOpenDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions src/gui/DatabaseOpenDialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
14 changes: 13 additions & 1 deletion src/gui/DatabaseOpenWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
*
Expand Down
8 changes: 5 additions & 3 deletions src/gui/DatabaseOpenWidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,13 @@ class DatabaseOpenWidget : public DialogyWidget
void clearForms();
void enterKey(const QString& pw, const QString& keyFile);
QSharedPointer<Database> database();
void resetQuickUnlock();
bool unlockingDatabase();

// Quick Unlock helper functions
bool isOnQuickUnlockScreen();
void triggerQuickUnlock();
void resetQuickUnlock();

signals:
void dialogFinished(bool accepted);

Expand All @@ -56,8 +60,6 @@ class DatabaseOpenWidget : public DialogyWidget
void hideEvent(QHideEvent* event) override;
QSharedPointer<CompositeKey> buildDatabaseKey();
void setUserInteractionLock(bool state);
// Quick Unlock helper functions
bool isOnQuickUnlockScreen();

const QScopedPointer<Ui::DatabaseOpenWidget> m_ui;
QSharedPointer<Database> m_db;
Expand Down
2 changes: 2 additions & 0 deletions tests/gui/TestGuiBrowser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<DatabaseTabWidget*>("tabWidget");
Expand Down
2 changes: 2 additions & 0 deletions tests/gui/TestGuiFdoSecrets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit eee25a1

Please sign in to comment.