diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 82d986c40..6b4c4c211 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -7117,14 +7117,6 @@ The following data is missing: Password quality - - Confirm Delete Wordlist - - - - Do you really want to delete the wordlist "%1"? - - Failed to delete wordlist @@ -7178,6 +7170,18 @@ Do you want to overwrite it? Excluded characters: "0", "1", "l", "I", "O", "|", "﹒", "B", "8", "G", "6" + + Warning: the chosen wordlist is smaller than the minimum recommended size! + + + + Confirm Remove Wordlist + + + + Do you really want to remove the wordlist "%1"? + + PasswordWidget @@ -9192,10 +9196,6 @@ This option is deprecated, use --set-key-file instead. Shortcut %1 conflicts with '%2'. Overwrite shortcut? - - Cannot generate valid passphrases because the wordlist is too short - - Encrypted files are not supported. @@ -9243,6 +9243,10 @@ This option is deprecated, use --set-key-file instead. Tags + + Warning: the chosen wordlist is smaller than the minimum recommended size! + + Fit diff --git a/src/cli/Diceware.cpp b/src/cli/Diceware.cpp index c2e3f9fee..9b5cc2ed6 100644 --- a/src/cli/Diceware.cpp +++ b/src/cli/Diceware.cpp @@ -68,11 +68,9 @@ int Diceware::execute(const QStringList& arguments) dicewareGenerator.setWordList(wordListFile); } - if (!dicewareGenerator.isValid()) { - // We already validated the word count input so if the generator is invalid, it - // must be because the word list is too small. - err << QObject::tr("Cannot generate valid passphrases because the wordlist is too short") << Qt::endl; - return EXIT_FAILURE; + // Show a warning if the wordlist is smaller than the recommended size + if (!dicewareGenerator.isWordListValid()) { + err << QObject::tr("Warning: the chosen wordlist is smaller than the minimum recommended size!") << Qt::endl; } QString password = dicewareGenerator.generatePassphrase(); diff --git a/src/core/PassphraseGenerator.cpp b/src/core/PassphraseGenerator.cpp index d8116c827..07e7a31bf 100644 --- a/src/core/PassphraseGenerator.cpp +++ b/src/core/PassphraseGenerator.cpp @@ -99,7 +99,7 @@ void PassphraseGenerator::setWordList(const QString& path) m_wordlist = wordset.toList(); - if (m_wordlist.size() < m_minimum_wordlist_length) { + if (!isWordListValid()) { qWarning("Wordlist is less than minimum acceptable size: %s", qPrintable(path)); } } @@ -117,8 +117,7 @@ void PassphraseGenerator::setWordSeparator(const QString& separator) QString PassphraseGenerator::generatePassphrase() const { - // In case there was an error loading the wordlist - if (!isValid() || m_wordlist.empty()) { + if (m_wordlist.isEmpty()) { return {}; } @@ -149,7 +148,7 @@ QString PassphraseGenerator::generatePassphrase() const return words.join(m_separator); } -bool PassphraseGenerator::isValid() const +bool PassphraseGenerator::isWordListValid() const { - return m_wordCount > 0 && m_wordlist.size() >= m_minimum_wordlist_length; + return m_wordlist.size() >= m_minWordListSize; } diff --git a/src/core/PassphraseGenerator.h b/src/core/PassphraseGenerator.h index 931dbe57e..5d404c990 100644 --- a/src/core/PassphraseGenerator.h +++ b/src/core/PassphraseGenerator.h @@ -40,7 +40,7 @@ public: void setWordCase(PassphraseWordCase wordCase); void setDefaultWordList(); void setWordSeparator(const QString& separator); - bool isValid() const; + bool isWordListValid() const; QString generatePassphrase() const; @@ -50,7 +50,7 @@ public: private: int m_wordCount; - int m_minimum_wordlist_length = 4000; + int m_minWordListSize = 1296; PassphraseWordCase m_wordCase; QString m_separator; QList m_wordlist; diff --git a/src/gui/PasswordGeneratorWidget.cpp b/src/gui/PasswordGeneratorWidget.cpp index f0f48f61a..d3dd7d699 100644 --- a/src/gui/PasswordGeneratorWidget.cpp +++ b/src/gui/PasswordGeneratorWidget.cpp @@ -65,7 +65,7 @@ PasswordGeneratorWidget::PasswordGeneratorWidget(QWidget* parent) connect(m_ui->buttonApply, SIGNAL(clicked()), SLOT(applyPassword())); connect(m_ui->buttonCopy, SIGNAL(clicked()), SLOT(copyPassword())); connect(m_ui->buttonGenerate, SIGNAL(clicked()), SLOT(regeneratePassword())); - connect(m_ui->buttonDeleteWordList, SIGNAL(clicked()), SLOT(deleteWordList())); + connect(m_ui->buttonDeleteWordList, SIGNAL(clicked()), SLOT(removeCustomWordList())); connect(m_ui->buttonAddWordList, SIGNAL(clicked()), SLOT(addWordList())); connect(m_ui->buttonClose, SIGNAL(clicked()), SIGNAL(closed())); @@ -115,6 +115,11 @@ PasswordGeneratorWidget::PasswordGeneratorWidget(QWidget* parent) m_ui->comboBoxWordList->addItem(fileName, path.absolutePath() + QDir::separator() + fileName); } + // Set color of wordlist warning + StateColorPalette statePalette; + auto color = statePalette.color(StateColorPalette::ColorRole::False); + m_ui->labelWordListWarning->setStyleSheet(QString("QLabel { color: %1; }").arg(color.name())); + loadSettings(); } @@ -257,9 +262,7 @@ void PasswordGeneratorWidget::regeneratePassword() m_ui->editNewPassword->setText(m_passwordGenerator->generatePassword()); } } else { - if (m_dicewareGenerator->isValid()) { - m_ui->editNewPassword->setText(m_dicewareGenerator->generatePassphrase()); - } + m_ui->editNewPassword->setText(m_dicewareGenerator->generatePassphrase()); } } @@ -379,33 +382,28 @@ bool PasswordGeneratorWidget::isPasswordGenerated() const return m_passwordGenerated; } -void PasswordGeneratorWidget::deleteWordList() +void PasswordGeneratorWidget::removeCustomWordList() { if (m_ui->comboBoxWordList->currentIndex() < m_firstCustomWordlistIndex) { return; } - QFile file(m_ui->comboBoxWordList->currentData().toString()); - if (!file.exists()) { - return; - } - + auto wordlist = m_ui->comboBoxWordList->currentText(); auto result = MessageBox::question(this, - tr("Confirm Delete Wordlist"), - tr("Do you really want to delete the wordlist \"%1\"?").arg(file.fileName()), - MessageBox::Delete | MessageBox::Cancel, + tr("Confirm Remove Wordlist"), + tr("Do you really want to remove the wordlist \"%1\"?").arg(wordlist), + MessageBox::Remove | MessageBox::Cancel, MessageBox::Cancel); - if (result != MessageBox::Delete) { - return; - } - if (!file.remove()) { - MessageBox::critical(this, tr("Failed to delete wordlist"), file.errorString()); - return; - } + if (result == MessageBox::Remove) { + QFile file(m_ui->comboBoxWordList->currentData().toString()); + if (file.exists() && !file.remove()) { + MessageBox::critical(this, tr("Failed to delete wordlist"), file.errorString()); + } - m_ui->comboBoxWordList->removeItem(m_ui->comboBoxWordList->currentIndex()); - updateGenerator(); + m_ui->comboBoxWordList->removeItem(m_ui->comboBoxWordList->currentIndex()); + updateGenerator(); + } } void PasswordGeneratorWidget::addWordList() @@ -589,11 +587,7 @@ void PasswordGeneratorWidget::updateGenerator() } m_passwordGenerator->setFlags(flags); - if (m_passwordGenerator->isValid()) { - m_ui->buttonGenerate->setEnabled(true); - } else { - m_ui->buttonGenerate->setEnabled(false); - } + m_ui->buttonGenerate->setEnabled(m_passwordGenerator->isValid()); } else { m_dicewareGenerator->setWordCase( static_cast(m_ui->wordCaseComboBox->currentData().toInt())); @@ -610,11 +604,8 @@ void PasswordGeneratorWidget::updateGenerator() m_dicewareGenerator->setWordSeparator(m_ui->editWordSeparator->text()); - if (m_dicewareGenerator->isValid()) { - m_ui->buttonGenerate->setEnabled(true); - } else { - m_ui->buttonGenerate->setEnabled(false); - } + m_ui->labelWordListWarning->setVisible(!m_dicewareGenerator->isWordListValid()); + m_ui->buttonGenerate->setEnabled(true); } regeneratePassword(); diff --git a/src/gui/PasswordGeneratorWidget.h b/src/gui/PasswordGeneratorWidget.h index 2f92a3eca..d7496365a 100644 --- a/src/gui/PasswordGeneratorWidget.h +++ b/src/gui/PasswordGeneratorWidget.h @@ -67,7 +67,7 @@ public slots: void applyPassword(); void copyPassword(); void setPasswordVisible(bool visible); - void deleteWordList(); + void removeCustomWordList(); void addWordList(); protected: diff --git a/src/gui/PasswordGeneratorWidget.ui b/src/gui/PasswordGeneratorWidget.ui index 47f197c81..eac5421cd 100644 --- a/src/gui/PasswordGeneratorWidget.ui +++ b/src/gui/PasswordGeneratorWidget.ui @@ -769,7 +769,113 @@ QProgressBar::chunk { + + + + + 0 + 0 + + + + Wordlist: + + + + + + + + + + 0 + 0 + + + + + 20 + 0 + + + + + + + + + + + Qt::Horizontal + + + + 40 + 20 + + + + + + + + + + Word Count: + + + spinBoxLength + + + + + + + Word Case: + + + + + + + + + 0 + 0 + + + + + + + + Qt::TabFocus + + + Delete selected wordlist + + + Delete selected wordlist + + + + + + + Qt::TabFocus + + + Add custom wordlist + + + Add custom wordlist + + + + + + QLayout::SetMinimumSize @@ -814,61 +920,7 @@ QProgressBar::chunk { - - - - Word Case: - - - - - - - Word Separator: - - - - - - - - - - 0 - 0 - - - - - - - - Qt::TabFocus - - - Delete selected wordlist - - - Delete selected wordlist - - - - - - - Qt::TabFocus - - - Add custom wordlist - - - Add custom wordlist - - - - - - + @@ -888,62 +940,23 @@ QProgressBar::chunk { - - + + - Word Count: - - - spinBoxLength + Word Separator: - - - - - - - 0 - 0 - - - - - 20 - 0 - - - - - - - - - - - Qt::Horizontal - - - - 40 - 20 - - - - - - - - - - - 0 - 0 - + + + + + 75 + true + - Wordlist: + Warning: the chosen wordlist is smaller than the minimum recommended size! diff --git a/tests/TestCli.cpp b/tests/TestCli.cpp index 9b2e5ab8f..e70b7b2df 100644 --- a/tests/TestCli.cpp +++ b/tests/TestCli.cpp @@ -1089,8 +1089,9 @@ void TestCli::testDiceware() } smallWordFile.close(); + // Ensure a warning is shown if the wordlist is too short execCmd(dicewareCmd, {"diceware", "-W", "11", "-w", smallWordFile.fileName()}); - QCOMPARE(m_stderr->readLine(), QByteArray("Cannot generate valid passphrases because the wordlist is too short\n")); + QVERIFY(m_stderr->readLine().length() > 0); } void TestCli::testEdit() diff --git a/tests/TestPassphraseGenerator.cpp b/tests/TestPassphraseGenerator.cpp index b27922a60..7b8dd89a4 100644 --- a/tests/TestPassphraseGenerator.cpp +++ b/tests/TestPassphraseGenerator.cpp @@ -34,7 +34,7 @@ void TestPassphraseGenerator::testWordCase() { PassphraseGenerator generator; generator.setWordSeparator(" "); - QVERIFY(generator.isValid()); + QVERIFY(generator.isWordListValid()); QString passphrase; passphrase = generator.generatePassphrase(); @@ -57,14 +57,14 @@ void TestPassphraseGenerator::testWordCase() void TestPassphraseGenerator::testUniqueEntriesInWordlist() { PassphraseGenerator generator; - // set the limit down, so we don;t have to do a very large file - generator.m_minimum_wordlist_length = 4; + // set the limit down, so we don't have to do a very large file + generator.m_minWordListSize = 4; // link to bad wordlist QString path = QString(KEEPASSX_TEST_DATA_DIR).append("/wordlists/bad_wordlist_with_duplicate_entries.wordlist"); - // setting will work, it creates the warning however, and isValid will fail + // setting will work, it creates the warning however, and isWordListValid will fail generator.setWordList(path); // so this fails - QVERIFY(!generator.isValid()); + QVERIFY(!generator.isWordListValid()); }