Skip to content

Commit

Permalink
Various minor bug fixes / enhancements
Browse files Browse the repository at this point in the history
* Fix issues when Config options were renamed
* Fix compile issues when using clang 10
* Rearrange database menu icons and import database menu icons
* Set minimum size of MainWindow to 800 to prevent search bar from hiding
* Fix not saving password generator options when closing the standalone generator
* Add headers to health check reports
* Don't show hidden content dots when notes are hidden but empty.

* Fix saving new database files in SMB shares on Windows, fixes keepassxreboot#4809

* Gracefully handle duplicate attachments :
Instead of bailing out with an error, prepend a random string to the name of duplicate attachment records. This prevents data loss from other programs that mishandled KDBX XML writing. Fixes keepassxreboot#2493

* Properly handle blocked import of signed KeeShare database, fixes keepassxreboot#4413
  • Loading branch information
droidmonkey committed Jun 4, 2020
1 parent e36cba7 commit c830f85
Show file tree
Hide file tree
Showing 19 changed files with 101 additions and 80 deletions.
6 changes: 3 additions & 3 deletions src/core/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static const QHash<Config::ConfigKey, ConfigDirective> configStrings = {
{Config::GlobalAutoTypeModifiers,{QS("GlobalAutoTypeModifiers"), Roaming, 0}},
{Config::TrackNonDataChanges,{QS("TrackNonDataChanges"), Roaming, false}},
{Config::FaviconDownloadTimeout,{QS("FaviconDownloadTimeout"), Roaming, 10}},
{Config::UpdateCheckMessageShown,{QS("UpdateCheckMessageShown"), Roaming, true}},
{Config::UpdateCheckMessageShown,{QS("UpdateCheckMessageShown"), Roaming, false}},
{Config::UseTouchID,{QS("UseTouchID"), Roaming, false}},

{Config::LastDatabases, {QS("LastDatabases"), Local, {}}},
Expand All @@ -102,6 +102,7 @@ static const QHash<Config::ConfigKey, ConfigDirective> configStrings = {
{Config::GUI_MonospaceNotes, {QS("GUI/MonospaceNotes"), Roaming, false}},
{Config::GUI_ApplicationTheme, {QS("GUI/ApplicationTheme"), Roaming, QS("auto")}},
{Config::GUI_CheckForUpdates, {QS("GUI/CheckForUpdates"), Roaming, true}},
{Config::GUI_CheckForUpdatesNextCheck, {QS("GUI/CheckForUpdatesNextCheck"), Local, 0}},
{Config::GUI_CheckForUpdatesIncludeBetas, {QS("GUI/CheckForUpdatesIncludeBetas"), Roaming, false}},

{Config::GUI_MainWindowGeometry, {QS("GUI/MainWindowGeometry"), Local, {}}},
Expand All @@ -111,7 +112,6 @@ static const QHash<Config::ConfigKey, ConfigDirective> configStrings = {
{Config::GUI_SplitterState, {QS("GUI/SplitterState"), Local, {}}},
{Config::GUI_PreviewSplitterState, {QS("GUI/PreviewSplitterState"), Local, {}}},
{Config::GUI_AutoTypeSelectDialogSize, {QS("GUI/AutoTypeSelectDialogSize"), Local, QSize(600, 250)}},
{Config::GUI_CheckForUpdatesNextCheck, {QS("GUI/AutoTypeSelectDialogSize"), Local, 0}},

// Security
{Config::Security_ClearClipboard, {QS("Security/ClearClipboard"), Roaming, true}},
Expand Down Expand Up @@ -185,13 +185,13 @@ static const QHash<Config::ConfigKey, ConfigDirective> configStrings = {
{Config::PasswordGenerator_EASCII, {QS("PasswordGenerator/EASCII"), Roaming, false}},
{Config::PasswordGenerator_AdvancedMode, {QS("PasswordGenerator/AdvancedMode"), Roaming, false}},
{Config::PasswordGenerator_SpecialChars, {QS("PasswordGenerator/SpecialChars"), Roaming, true}},
{Config::PasswordGenerator_AdditionalChars, {QS("PasswordGenerator/AdditionalChars"), Roaming, true}},
{Config::PasswordGenerator_Braces, {QS("PasswordGenerator/Braces"), Roaming, false}},
{Config::PasswordGenerator_Punctuation, {QS("PasswordGenerator/Punctuation"), Roaming, false}},
{Config::PasswordGenerator_Quotes, {QS("PasswordGenerator/Quotes"), Roaming, false}},
{Config::PasswordGenerator_Dashes, {QS("PasswordGenerator/Dashes"), Roaming, false}},
{Config::PasswordGenerator_Math, {QS("PasswordGenerator/Math"), Roaming, false}},
{Config::PasswordGenerator_Logograms, {QS("PasswordGenerator/Logograms"), Roaming, false}},
{Config::PasswordGenerator_AdditionalChars, {QS("PasswordGenerator/AdditionalChars"), Roaming, {}}},
{Config::PasswordGenerator_ExcludedChars, {QS("PasswordGenerator/ExcludedChars"), Roaming, {}}},
{Config::PasswordGenerator_ExcludeAlike, {QS("PasswordGenerator/ExcludeAlike"), Roaming, true}},
{Config::PasswordGenerator_EnsureEvery, {QS("PasswordGenerator/EnsureEvery"), Roaming, true}},
Expand Down
7 changes: 4 additions & 3 deletions src/core/Database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,13 @@ bool Database::saveAs(const QString& filePath, QString* error, bool atomic, bool
setReadOnly(false);
m_fileWatcher->stop();

auto& canonicalFilePath = QFileInfo::exists(filePath) ? QFileInfo(filePath).canonicalFilePath() : filePath;
bool ok = AsyncTask::runAndWaitForFuture([&] { return performSave(canonicalFilePath, error, atomic, backup); });
QFileInfo fileInfo(filePath);
auto realFilePath = fileInfo.exists() ? fileInfo.canonicalFilePath() : fileInfo.absoluteFilePath();
bool ok = AsyncTask::runAndWaitForFuture([&] { return performSave(realFilePath, error, atomic, backup); });
if (ok) {
markAsClean();
setFilePath(filePath);
m_fileWatcher->start(canonicalFilePath, 30, 1);
m_fileWatcher->start(realFilePath, 30, 1);
} else {
// Saving failed, don't rewatch file since it does not represent our database
markAsModified();
Expand Down
2 changes: 1 addition & 1 deletion src/core/PasswordHealth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ QSharedPointer<PasswordHealth> HealthChecker::evaluate(const Entry* entry) const
for (int i = 0; i < used.size(); ++i) {
health->addScoreDetails(used[i]);
if (i == 19) {
health->addScoreDetails(QStringLiteral("..."));
health->addScoreDetails("");
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/crypto/ssh/bcrypt_pbkdf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ bcrypt_hash(const quint8* sha2pass, const quint8* sha2salt, quint8* out)
cdata[i] = Blowfish_stream2word(ciphertext, sizeof(ciphertext),
&j);
for (i = 0; i < 64; i++)
blf_enc(&state, cdata, sizeof(cdata) / sizeof(uint64_t));
blf_enc(&state, cdata, BCRYPT_WORDS / 2);

/* copy out */
for (i = 0; i < BCRYPT_WORDS; i++) {
Expand Down
10 changes: 6 additions & 4 deletions src/format/KdbxXmlReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -875,11 +875,13 @@ QPair<QString, QString> KdbxXmlReader::parseEntryBinary(Entry* entry)
}

if (keySet && valueSet) {
if (entry->attachments()->hasKey(key)) {
raiseError(tr("Duplicate attachment found"));
} else {
entry->attachments()->set(key, value);
if (entry->attachments()->hasKey(key) && entry->attachments()->value(key) != value) {
// NOTE: This only impacts KDBX 3.x databases
// Prepend a random string to the key to make it unique and prevent data loss
key = key.prepend(QUuid::createUuid().toString().mid(1, 8) + "_");
qWarning("Duplicate attachment name found, renamed to: %s", qPrintable(key));
}
entry->attachments()->set(key, value);
} else {
raiseError(tr("Entry binary key or value missing"));
}
Expand Down
25 changes: 11 additions & 14 deletions src/gui/MainWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ MainWindow::MainWindow()
m_ui->actionDatabaseSaveBackup->setIcon(resources()->icon("document-save-copy"));
m_ui->actionDatabaseClose->setIcon(resources()->icon("document-close"));
m_ui->actionReports->setIcon(resources()->icon("reports"));
m_ui->actionChangeDatabaseSettings->setIcon(resources()->icon("document-edit"));
m_ui->actionDatabaseSettings->setIcon(resources()->icon("document-edit"));
m_ui->actionChangeMasterKey->setIcon(resources()->icon("database-change-key"));
m_ui->actionLockDatabases->setIcon(resources()->icon("database-lock"));
m_ui->actionQuit->setIcon(resources()->icon("application-exit"));
Expand Down Expand Up @@ -413,7 +413,7 @@ MainWindow::MainWindow()
connect(m_ui->actionDatabaseMerge, SIGNAL(triggered()), m_ui->tabWidget, SLOT(mergeDatabase()));
connect(m_ui->actionChangeMasterKey, SIGNAL(triggered()), m_ui->tabWidget, SLOT(changeMasterKey()));
connect(m_ui->actionReports, SIGNAL(triggered()), m_ui->tabWidget, SLOT(changeReports()));
connect(m_ui->actionChangeDatabaseSettings, SIGNAL(triggered()), m_ui->tabWidget, SLOT(changeDatabaseSettings()));
connect(m_ui->actionDatabaseSettings, SIGNAL(triggered()), m_ui->tabWidget, SLOT(changeDatabaseSettings()));
connect(m_ui->actionImportCsv, SIGNAL(triggered()), m_ui->tabWidget, SLOT(importCsv()));
connect(m_ui->actionImportKeePass1, SIGNAL(triggered()), m_ui->tabWidget, SLOT(importKeePass1Database()));
connect(m_ui->actionImportOpVault, SIGNAL(triggered()), m_ui->tabWidget, SLOT(importOpVaultDatabase()));
Expand Down Expand Up @@ -456,8 +456,11 @@ MainWindow::MainWindow()
m_actionMultiplexer.connect(m_ui->actionGroupDownloadFavicons, SIGNAL(triggered()), SLOT(downloadAllFavicons()));

connect(m_ui->actionSettings, SIGNAL(toggled(bool)), SLOT(switchToSettings(bool)));
connect(m_ui->actionPasswordGenerator, SIGNAL(toggled(bool)), SLOT(switchToPasswordGen(bool)));
connect(m_ui->passwordGeneratorWidget, SIGNAL(closePasswordGenerator()), SLOT(closePasswordGen()));
connect(m_ui->actionPasswordGenerator, SIGNAL(toggled(bool)), SLOT(togglePasswordGenerator(bool)));
connect(m_ui->passwordGeneratorWidget, &PasswordGeneratorWidget::closed, this, [this] {
togglePasswordGenerator(false);
});
m_ui->passwordGeneratorWidget->setStandaloneMode(true);

connect(m_ui->welcomeWidget, SIGNAL(newDatabase()), SLOT(switchToNewDatabase()));
connect(m_ui->welcomeWidget, SIGNAL(openDatabase()), SLOT(switchToOpenDatabase()));
Expand Down Expand Up @@ -719,7 +722,7 @@ void MainWindow::setMenuActionState(DatabaseWidget::Mode mode)
&& !recycleBinSelected);
m_ui->actionChangeMasterKey->setEnabled(true);
m_ui->actionReports->setEnabled(true);
m_ui->actionChangeDatabaseSettings->setEnabled(true);
m_ui->actionDatabaseSettings->setEnabled(true);
m_ui->actionDatabaseSave->setEnabled(m_ui->tabWidget->canSave());
m_ui->actionDatabaseSaveAs->setEnabled(true);
m_ui->actionDatabaseSaveBackup->setEnabled(true);
Expand Down Expand Up @@ -775,7 +778,7 @@ void MainWindow::setMenuActionState(DatabaseWidget::Mode mode)

m_ui->actionChangeMasterKey->setEnabled(false);
m_ui->actionReports->setEnabled(false);
m_ui->actionChangeDatabaseSettings->setEnabled(false);
m_ui->actionDatabaseSettings->setEnabled(false);
m_ui->actionDatabaseSave->setEnabled(false);
m_ui->actionDatabaseSaveAs->setEnabled(false);
m_ui->actionDatabaseSaveBackup->setEnabled(false);
Expand Down Expand Up @@ -804,7 +807,7 @@ void MainWindow::setMenuActionState(DatabaseWidget::Mode mode)

m_ui->actionChangeMasterKey->setEnabled(false);
m_ui->actionReports->setEnabled(false);
m_ui->actionChangeDatabaseSettings->setEnabled(false);
m_ui->actionDatabaseSettings->setEnabled(false);
m_ui->actionDatabaseSave->setEnabled(false);
m_ui->actionDatabaseSaveAs->setEnabled(false);
m_ui->actionDatabaseSaveBackup->setEnabled(false);
Expand Down Expand Up @@ -990,24 +993,18 @@ void MainWindow::switchToSettings(bool enabled)
}
}

void MainWindow::switchToPasswordGen(bool enabled)
void MainWindow::togglePasswordGenerator(bool enabled)
{
if (enabled) {
m_ui->passwordGeneratorWidget->loadSettings();
m_ui->passwordGeneratorWidget->regeneratePassword();
m_ui->stackedWidget->setCurrentIndex(PasswordGeneratorScreen);
m_ui->passwordGeneratorWidget->setStandaloneMode(true);
} else {
m_ui->passwordGeneratorWidget->saveSettings();
switchToDatabases();
}
}

void MainWindow::closePasswordGen()
{
switchToPasswordGen(false);
}

void MainWindow::switchToNewDatabase()
{
m_ui->tabWidget->newDatabase();
Expand Down
3 changes: 1 addition & 2 deletions src/gui/MainWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,13 @@ private slots:
void openKeyboardShortcuts();
void switchToDatabases();
void switchToSettings(bool enabled);
void switchToPasswordGen(bool enabled);
void togglePasswordGenerator(bool enabled);
void switchToNewDatabase();
void switchToOpenDatabase();
void switchToDatabaseFile(const QString& file);
void switchToKeePass1Database();
void switchToOpVaultDatabase();
void switchToCsvImport();
void closePasswordGen();
void databaseStatusChanged(DatabaseWidget* dbWidget);
void databaseTabChanged(int tabIndex);
void openRecentDatabase(QAction* action);
Expand Down
24 changes: 15 additions & 9 deletions src/gui/MainWindow.ui
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
<height>600</height>
</rect>
</property>
<property name="minimumSize">
<size>
<width>800</width>
<height>0</height>
</size>
</property>
<property name="windowTitle">
<string notr="true">KeePassXC</string>
</property>
Expand Down Expand Up @@ -229,9 +235,9 @@
<property name="title">
<string>&amp;Import</string>
</property>
<addaction name="actionImportKeePass1"/>
<addaction name="actionImportOpVault"/>
<addaction name="actionImportCsv"/>
<addaction name="actionImportOpVault"/>
<addaction name="actionImportKeePass1"/>
</widget>
<widget class="QMenu" name="menuExport">
<property name="title">
Expand All @@ -248,9 +254,9 @@
<addaction name="actionDatabaseSaveBackup"/>
<addaction name="actionDatabaseClose"/>
<addaction name="separator"/>
<addaction name="actionChangeMasterKey"/>
<addaction name="actionReports"/>
<addaction name="actionChangeDatabaseSettings"/>
<addaction name="actionDatabaseSettings"/>
<addaction name="actionChangeMasterKey"/>
<addaction name="separator"/>
<addaction name="actionDatabaseMerge"/>
<addaction name="menuImport"/>
Expand Down Expand Up @@ -381,9 +387,9 @@
<addaction name="actionEntryCopyURL"/>
<addaction name="actionEntryAutoType"/>
<addaction name="separator"/>
<addaction name="actionPasswordGenerator"/>
<addaction name="actionLockDatabases"/>
<addaction name="separator"/>
<addaction name="actionPasswordGenerator"/>
<addaction name="actionSettings"/>
<addaction name="separator"/>
</widget>
Expand Down Expand Up @@ -550,7 +556,7 @@
<bool>false</bool>
</property>
<property name="text">
<string>&amp;Reports...</string>
<string>&amp;Database Reports...</string>
</property>
<property name="toolTip">
<string>Statistics, health check, etc.</string>
Expand All @@ -559,7 +565,7 @@
<enum>QAction::NoRole</enum>
</property>
</action>
<action name="actionChangeDatabaseSettings">
<action name="actionDatabaseSettings">
<property name="enabled">
<bool>false</bool>
</property>
Expand Down Expand Up @@ -711,15 +717,15 @@
<bool>false</bool>
</property>
<property name="text">
<string>&amp;Export to CSV File…</string>
<string>&amp;CSV File…</string>
</property>
</action>
<action name="actionExportHtml">
<property name="enabled">
<bool>false</bool>
</property>
<property name="text">
<string>&amp;Export to HTML File…</string>
<string>&amp;HTML File…</string>
</property>
</action>
<action name="actionImportKeePass1">
Expand Down
29 changes: 17 additions & 12 deletions src/gui/PasswordGeneratorWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ PasswordGeneratorWidget::PasswordGeneratorWidget(QWidget* parent)
m_ui->buttonCopy->setIcon(resources()->icon("clipboard-text"));
m_ui->buttonClose->setShortcut(Qt::Key_Escape);

m_ui->clearInclude->setIcon(resources()->icon("edit-clear-locationbar-rtl"));
m_ui->editAdditionalChars->addAction(m_ui->clearInclude, QLineEdit::TrailingPosition);
m_ui->clearInclude->setVisible(false);

m_ui->clearExclude->setIcon(resources()->icon("edit-clear-locationbar-rtl"));
m_ui->editExcludedChars->addAction(m_ui->clearExclude, QLineEdit::TrailingPosition);
m_ui->clearExclude->setVisible(false);

connect(m_ui->editNewPassword, SIGNAL(textChanged(QString)), SLOT(updateButtonsEnabled(QString)));
connect(m_ui->editNewPassword, SIGNAL(textChanged(QString)), SLOT(updatePasswordStrength(QString)));
connect(m_ui->buttonAdvancedMode, SIGNAL(toggled(bool)), SLOT(setAdvancedMode(bool)));
Expand All @@ -53,7 +61,9 @@ 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->buttonClose, SIGNAL(clicked()), SIGNAL(closePasswordGenerator()));
connect(m_ui->buttonClose, SIGNAL(clicked()), SIGNAL(closed()));
connect(m_ui->clearInclude, SIGNAL(triggered(bool)), m_ui->editAdditionalChars, SLOT(clear()));
connect(m_ui->clearExclude, SIGNAL(triggered(bool)), m_ui->editExcludedChars, SLOT(clear()));

connect(m_ui->sliderLength, SIGNAL(valueChanged(int)), SLOT(passwordLengthChanged(int)));
connect(m_ui->spinBoxLength, SIGNAL(valueChanged(int)), SLOT(passwordLengthChanged(int)));
Expand Down Expand Up @@ -110,7 +120,7 @@ PasswordGeneratorWidget* PasswordGeneratorWidget::popupGenerator(QWidget* parent
pwGenerator->setWindowFlags(Qt::Dialog | Qt::MSWindowsFixedSizeDialogHint);
pwGenerator->setStandaloneMode(false);

connect(pwGenerator, SIGNAL(closePasswordGenerator()), pwGenerator, SLOT(deleteLater()));
connect(pwGenerator, SIGNAL(closed()), pwGenerator, SLOT(deleteLater()));

pwGenerator->show();
pwGenerator->raise();
Expand Down Expand Up @@ -180,6 +190,7 @@ void PasswordGeneratorWidget::saveSettings()
config()->set(Config::PasswordGenerator_Dashes, m_ui->checkBoxDashes->isChecked());
config()->set(Config::PasswordGenerator_Math, m_ui->checkBoxMath->isChecked());
config()->set(Config::PasswordGenerator_Logograms, m_ui->checkBoxLogograms->isChecked());
config()->set(Config::PasswordGenerator_AdditionalChars, m_ui->editAdditionalChars->text());
config()->set(Config::PasswordGenerator_ExcludedChars, m_ui->editExcludedChars->text());
config()->set(Config::PasswordGenerator_ExcludeAlike, m_ui->checkBoxExcludeAlike->isChecked());
config()->set(Config::PasswordGenerator_EnsureEvery, m_ui->checkBoxEnsureEvery->isChecked());
Expand Down Expand Up @@ -220,15 +231,6 @@ QString PasswordGeneratorWidget::getGeneratedPassword()
return m_ui->editNewPassword->text();
}

void PasswordGeneratorWidget::keyPressEvent(QKeyEvent* e)
{
if (e->key() == Qt::Key_Escape && m_standalone) {
emit closePasswordGenerator();
} else {
e->ignore();
}
}

void PasswordGeneratorWidget::regeneratePassword()
{
if (m_ui->tabWidget->currentIndex() == Password) {
Expand Down Expand Up @@ -273,7 +275,7 @@ void PasswordGeneratorWidget::applyPassword()
{
saveSettings();
emit appliedPassword(m_ui->editNewPassword->text());
emit closePasswordGenerator();
emit closed();
}

void PasswordGeneratorWidget::copyPassword()
Expand Down Expand Up @@ -534,6 +536,9 @@ void PasswordGeneratorWidget::updateGenerator()
} else {
m_ui->buttonGenerate->setEnabled(false);
}

m_ui->clearInclude->setVisible(!m_ui->editAdditionalChars->text().isEmpty());
m_ui->clearExclude->setVisible(!m_ui->editExcludedChars->text().isEmpty());
} else {
m_dicewareGenerator->setWordCase(
static_cast<PassphraseGenerator::PassphraseWordCase>(m_ui->wordCaseComboBox->currentData().toInt()));
Expand Down
5 changes: 1 addition & 4 deletions src/gui/PasswordGeneratorWidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public slots:

signals:
void appliedPassword(const QString& password);
void closePasswordGenerator();
void closed();

private slots:
void updateButtonsEnabled(const QString& password);
Expand All @@ -87,9 +87,6 @@ private slots:
const QScopedPointer<PasswordGenerator> m_passwordGenerator;
const QScopedPointer<PassphraseGenerator> m_dicewareGenerator;
const QScopedPointer<Ui::PasswordGeneratorWidget> m_ui;

protected:
void keyPressEvent(QKeyEvent* e) override;
};

#endif // KEEPASSX_PASSWORDGENERATORWIDGET_H
Loading

0 comments on commit c830f85

Please sign in to comment.