diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 59a8e940d..91467de50 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -1856,11 +1856,11 @@ Are you sure you want to continue without a password? - You must enter a stronger password to protect your database. + This is a weak password! For better protection of your secrets, you should choose a stronger password. - This is a weak password! For better protection of your secrets, you should choose a stronger password. + The provided password does not meet the minimum quality requirement. diff --git a/src/gui/databasekey/KeyComponentWidget.cpp b/src/gui/databasekey/KeyComponentWidget.cpp index f49c10e1e..1fad34427 100644 --- a/src/gui/databasekey/KeyComponentWidget.cpp +++ b/src/gui/databasekey/KeyComponentWidget.cpp @@ -73,6 +73,10 @@ KeyComponentWidget::Page KeyComponentWidget::visiblePage() const void KeyComponentWidget::updateAddStatus(bool added) { + if (m_ui->stackedWidget->currentIndex() == Page::Edit) { + emit editCanceled(); + } + if (added) { m_ui->stackedWidget->setCurrentIndex(Page::LeaveOrRemove); } else { @@ -101,12 +105,6 @@ void KeyComponentWidget::cancelEdit() emit editCanceled(); } -void KeyComponentWidget::showEvent(QShowEvent* event) -{ - resetComponentEditWidget(); - QWidget::showEvent(event); -} - void KeyComponentWidget::resetComponentEditWidget() { if (!m_componentWidget || static_cast(m_ui->stackedWidget->currentIndex()) == Page::Edit) { diff --git a/src/gui/databasekey/KeyComponentWidget.h b/src/gui/databasekey/KeyComponentWidget.h index 9ea53da96..d207e494d 100644 --- a/src/gui/databasekey/KeyComponentWidget.h +++ b/src/gui/databasekey/KeyComponentWidget.h @@ -104,9 +104,6 @@ signals: void editCanceled(); void componentRemovalRequested(); -protected: - void showEvent(QShowEvent* event) override; - private slots: void updateAddStatus(bool added); void doAdd(); diff --git a/src/gui/databasekey/PasswordEditWidget.cpp b/src/gui/databasekey/PasswordEditWidget.cpp index 04d63c43d..e2c34be07 100644 --- a/src/gui/databasekey/PasswordEditWidget.cpp +++ b/src/gui/databasekey/PasswordEditWidget.cpp @@ -27,6 +27,9 @@ PasswordEditWidget::PasswordEditWidget(QWidget* parent) , m_compUi(new Ui::PasswordEditWidget()) { initComponent(); + + // Explicitly clear password on cancel + connect(this, &PasswordEditWidget::editCanceled, this, [this] { setPassword({}); }); } PasswordEditWidget::~PasswordEditWidget() = default; @@ -59,7 +62,7 @@ bool PasswordEditWidget::isPasswordVisible() const bool PasswordEditWidget::isEmpty() const { - return (visiblePage() == Page::Edit) && m_compUi->enterPasswordEdit->text().isEmpty(); + return visiblePage() != Page::Edit || m_compUi->enterPasswordEdit->text().isEmpty(); } PasswordHealth::Quality PasswordEditWidget::getPasswordQuality() const @@ -83,8 +86,7 @@ void PasswordEditWidget::initComponentEditWidget(QWidget* widget) { Q_UNUSED(widget); Q_ASSERT(m_compEditWidget); - m_compUi->enterPasswordEdit->setFocus(); - + setFocusProxy(m_compUi->enterPasswordEdit); m_compUi->enterPasswordEdit->setQualityVisible(true); m_compUi->repeatPasswordEdit->setQualityVisible(false); } @@ -103,16 +105,6 @@ void PasswordEditWidget::initComponent() "

Good passwords are long and unique. KeePassXC can generate one for you.

")); } -void PasswordEditWidget::hideEvent(QHideEvent* event) -{ - if (!isVisible() && m_compUi->enterPasswordEdit) { - m_compUi->enterPasswordEdit->setText(""); - m_compUi->repeatPasswordEdit->setText(""); - } - - QWidget::hideEvent(event); -} - bool PasswordEditWidget::validate(QString& errorMessage) const { if (m_compUi->enterPasswordEdit->text() != m_compUi->repeatPasswordEdit->text()) { diff --git a/src/gui/databasekey/PasswordEditWidget.h b/src/gui/databasekey/PasswordEditWidget.h index 5e7bb28d6..14dfe1d81 100644 --- a/src/gui/databasekey/PasswordEditWidget.h +++ b/src/gui/databasekey/PasswordEditWidget.h @@ -47,7 +47,6 @@ protected: QWidget* componentEditWidget() override; void initComponentEditWidget(QWidget* widget) override; void initComponent() override; - void hideEvent(QHideEvent* event) override; private slots: void setPassword(const QString& password); diff --git a/src/gui/dbsettings/DatabaseSettingsDialog.cpp b/src/gui/dbsettings/DatabaseSettingsDialog.cpp index 14a303262..4a325ca42 100644 --- a/src/gui/dbsettings/DatabaseSettingsDialog.cpp +++ b/src/gui/dbsettings/DatabaseSettingsDialog.cpp @@ -92,36 +92,32 @@ DatabaseSettingsDialog::DatabaseSettingsDialog(QWidget* parent) scrollArea->setSizeAdjustPolicy(QScrollArea::AdjustToContents); scrollArea->setWidgetResizable(true); scrollArea->setWidget(m_databaseKeyWidget); - m_securityTabWidget->addTab(scrollArea, tr("Database Credentials")); + m_securityTabWidget->addTab(scrollArea, tr("Database Credentials")); m_securityTabWidget->addTab(m_encryptionWidget, tr("Encryption Settings")); + m_securityTabWidget->setCurrentIndex(0); m_ui->categoryList->addCategory(tr("Remote Sync"), icons()->icon("remote-sync")); m_ui->stackedWidget->addWidget(m_remoteWidget); -#if defined(WITH_XC_KEESHARE) - addSettingsPage(new DatabaseSettingsPageKeeShare()); -#endif - -#if defined(WITH_XC_FDOSECRETS) - addSettingsPage(new DatabaseSettingsPageFdoSecrets()); -#endif - - m_ui->stackedWidget->setCurrentIndex(0); - m_securityTabWidget->setCurrentIndex(0); - - connect(m_securityTabWidget, SIGNAL(currentChanged(int)), SLOT(pageChanged())); - connect(m_ui->categoryList, SIGNAL(categoryChanged(int)), m_ui->stackedWidget, SLOT(setCurrentIndex(int))); - #ifdef WITH_XC_BROWSER m_ui->categoryList->addCategory(tr("Browser Integration"), icons()->icon("internet-web-browser")); m_ui->stackedWidget->addWidget(m_browserWidget); #endif +#ifdef WITH_XC_KEESHARE + addSettingsPage(new DatabaseSettingsPageKeeShare()); +#endif + +#ifdef WITH_XC_FDOSECRETS + addSettingsPage(new DatabaseSettingsPageFdoSecrets()); +#endif + m_ui->categoryList->addCategory(tr("Maintenance"), icons()->icon("hammer-wrench")); m_ui->stackedWidget->addWidget(m_maintenanceWidget); - pageChanged(); + m_ui->stackedWidget->setCurrentIndex(0); + connect(m_ui->categoryList, SIGNAL(categoryChanged(int)), m_ui->stackedWidget, SLOT(setCurrentIndex(int))); } DatabaseSettingsDialog::~DatabaseSettingsDialog() = default; @@ -171,21 +167,29 @@ void DatabaseSettingsDialog::showRemoteSettings() void DatabaseSettingsDialog::save() { if (!m_generalWidget->save()) { + m_ui->categoryList->setCurrentCategory(0); return; } if (!m_databaseKeyWidget->save()) { + m_ui->categoryList->setCurrentCategory(1); + m_securityTabWidget->setCurrentIndex(0); return; } if (!m_encryptionWidget->save()) { + m_ui->categoryList->setCurrentCategory(1); + m_securityTabWidget->setCurrentIndex(1); return; } if (!m_remoteWidget->save()) { + m_ui->categoryList->setCurrentCategory(2); return; } + // Browser settings don't have anything to save + for (const ExtraPage& extraPage : asConst(m_extraPages)) { extraPage.saveSettings(); } @@ -195,10 +199,13 @@ void DatabaseSettingsDialog::save() void DatabaseSettingsDialog::reject() { + m_generalWidget->discard(); + m_databaseKeyWidget->discard(); + m_encryptionWidget->discard(); + m_remoteWidget->discard(); +#ifdef WITH_XC_BROWSER + m_browserWidget->discard(); +#endif + emit editFinished(false); } - -void DatabaseSettingsDialog::pageChanged() -{ - m_ui->stackedWidget->currentIndex(); -} diff --git a/src/gui/dbsettings/DatabaseSettingsDialog.h b/src/gui/dbsettings/DatabaseSettingsDialog.h index 9f04ecf39..a7772f329 100644 --- a/src/gui/dbsettings/DatabaseSettingsDialog.h +++ b/src/gui/dbsettings/DatabaseSettingsDialog.h @@ -70,7 +70,6 @@ signals: private slots: void save(); void reject(); - void pageChanged(); private: enum Page diff --git a/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp b/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp index 1de8e6a9f..f631f021d 100644 --- a/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp +++ b/src/gui/dbsettings/DatabaseSettingsWidgetDatabaseKey.cpp @@ -79,28 +79,30 @@ void DatabaseSettingsWidgetDatabaseKey::load(QSharedPointer db) // database has no key, we are about to add a new one m_passwordEditWidget->changeVisiblePage(KeyComponentWidget::Page::Edit); m_passwordEditWidget->setPasswordVisible(true); - } - - bool hasAdditionalKeys = false; - for (const auto& key : m_db->key()->keys()) { - if (key->uuid() == PasswordKey::UUID) { - m_passwordEditWidget->setComponentAdded(true); - } else if (key->uuid() == FileKey::UUID) { - m_keyFileEditWidget->setComponentAdded(true); - hasAdditionalKeys = true; + // Focus won't work until the UI settles + QTimer::singleShot(0, m_passwordEditWidget, SLOT(setFocus())); + } else { + bool hasAdditionalKeys = false; + for (const auto& key : m_db->key()->keys()) { + if (key->uuid() == PasswordKey::UUID) { + m_passwordEditWidget->setComponentAdded(true); + } else if (key->uuid() == FileKey::UUID) { + m_keyFileEditWidget->setComponentAdded(true); + hasAdditionalKeys = true; + } } - } #ifdef WITH_XC_YUBIKEY - for (const auto& key : m_db->key()->challengeResponseKeys()) { - if (key->uuid() == ChallengeResponseKey::UUID) { - m_yubiKeyEditWidget->setComponentAdded(true); - hasAdditionalKeys = true; + for (const auto& key : m_db->key()->challengeResponseKeys()) { + if (key->uuid() == ChallengeResponseKey::UUID) { + m_yubiKeyEditWidget->setComponentAdded(true); + hasAdditionalKeys = true; + } } - } #endif - setAdditionalKeyOptionsVisible(hasAdditionalKeys); + setAdditionalKeyOptionsVisible(hasAdditionalKeys); + } connect(m_passwordEditWidget->findChild("removeButton"), SIGNAL(clicked()), SLOT(markDirty())); connect(m_keyFileEditWidget->findChild("removeButton"), SIGNAL(clicked()), SLOT(markDirty())); @@ -177,31 +179,32 @@ bool DatabaseSettingsWidgetDatabaseKey::save() return false; } - // Show warning if database password is weak - if (!m_passwordEditWidget->isEmpty() - && m_passwordEditWidget->getPasswordQuality() < PasswordHealth::Quality::Good) { - auto dialogResult = MessageBox::warning(this, - tr("Weak password"), - tr("This is a weak password! For better protection of your secrets, " - "you should choose a stronger password."), - MessageBox::ContinueWithWeakPass | MessageBox::Cancel, - MessageBox::Cancel); - - if (dialogResult == MessageBox::Cancel) { + if (!m_passwordEditWidget->isEmpty()) { + // Prevent setting password with a quality less than the minimum required + auto minQuality = qBound(0, config()->get(Config::Security_DatabasePasswordMinimumQuality).toInt(), 4); + if (m_passwordEditWidget->getPasswordQuality() < static_cast(minQuality)) { + MessageBox::critical(this, + tr("Weak password"), + tr("The provided password does not meet the minimum quality requirement."), + MessageBox::Ok, + MessageBox::Ok); return false; } - } - // If enforced in the config file, deny users from continuing with a weak password - auto minQuality = - static_cast(config()->get(Config::Security_DatabasePasswordMinimumQuality).toInt()); - if (!m_passwordEditWidget->isEmpty() && m_passwordEditWidget->getPasswordQuality() < minQuality) { - MessageBox::critical(this, - tr("Weak password"), - tr("You must enter a stronger password to protect your database."), - MessageBox::Ok, - MessageBox::Ok); - return false; + // Show warning if database password is weak or poor + if (m_passwordEditWidget->getPasswordQuality() < PasswordHealth::Quality::Good) { + auto dialogResult = + MessageBox::warning(this, + tr("Weak password"), + tr("This is a weak password! For better protection of your secrets, " + "you should choose a stronger password."), + MessageBox::ContinueWithWeakPass | MessageBox::Cancel, + MessageBox::Cancel); + + if (dialogResult == MessageBox::Cancel) { + return false; + } + } } if (!addToCompositeKey(m_keyFileEditWidget, newKey, oldFileKey)) { @@ -232,11 +235,16 @@ bool DatabaseSettingsWidgetDatabaseKey::save() m_db->markAsModified(); } + // Reset fields + initialize(); + return true; } void DatabaseSettingsWidgetDatabaseKey::discard() { + // Reset fields + initialize(); emit editFinished(false); } diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index 2408a05de..03cb5e347 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -1607,12 +1607,20 @@ void TestGui::testDatabaseSettings() passwordWidgets[0]->setText("b"); passwordWidgets[1]->setText("b"); - // Cancel password change + // Toggle between tabs to ensure the password remains + securityTabWidget->setCurrentIndex(1); + QApplication::processEvents(); + securityTabWidget->setCurrentIndex(0); + QApplication::processEvents(); + QCOMPARE(passwordWidgets[0]->text(), QString("b")); + + // Cancel password change and confirm password is cleared auto cancelPasswordButton = passwordEditWidget->findChild("cancelButton"); QVERIFY(cancelPasswordButton); QTest::mouseClick(cancelPasswordButton, Qt::LeftButton); QApplication::processEvents(); QVERIFY(!passwordWidgets[0]->isVisible()); + QCOMPARE(passwordWidgets[0]->text(), QString("")); QVERIFY(editPasswordButton->isVisible()); // Switch to encryption tab and interact with various settings