From d6324feafd8d138819534520f3193a5fe18a691e Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Mon, 25 Mar 2019 08:40:55 -0400 Subject: [PATCH 01/19] Fix base64 check missing '/' as valid character * Issue introduced in 558cb3d * Corrects loading of legacy KeePass Key Files that included a '/' in their data section. Fix #2863 and Fix #2834 --- src/core/Tools.cpp | 2 +- tests/TestTools.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/Tools.cpp b/src/core/Tools.cpp index 09938b82..46cde95b 100644 --- a/src/core/Tools.cpp +++ b/src/core/Tools.cpp @@ -205,7 +205,7 @@ namespace Tools bool isBase64(const QByteArray& ba) { - constexpr auto pattern = R"(^(?:[a-z0-9+]{4})*(?:[a-z0-9+]{3}=|[a-z0-9+]{2}==)?$)"; + constexpr auto pattern = R"(^(?:[a-z0-9+/]{4})*(?:[a-z0-9+/]{3}=|[a-z0-9+/]{2}==)?$)"; QRegExp regexp(pattern, Qt::CaseInsensitive, QRegExp::RegExp2); QString base64 = QString::fromLatin1(ba.constData(), ba.size()); diff --git a/tests/TestTools.cpp b/tests/TestTools.cpp index de5a80c0..100eb630 100644 --- a/tests/TestTools.cpp +++ b/tests/TestTools.cpp @@ -59,6 +59,7 @@ void TestTools::testIsBase64() QVERIFY(Tools::isBase64(QByteArray("12=="))); QVERIFY(Tools::isBase64(QByteArray("abcd9876MN=="))); QVERIFY(Tools::isBase64(QByteArray("abcd9876DEFGhijkMNO="))); + QVERIFY(Tools::isBase64(QByteArray("abcd987/DEFGh+jk/NO="))); QVERIFY(not Tools::isBase64(QByteArray("abcd123=="))); QVERIFY(not Tools::isBase64(QByteArray("abc_"))); QVERIFY(not Tools::isBase64(QByteArray("123"))); From e7862910869b45d4391c30e0194b774d77cb4606 Mon Sep 17 00:00:00 2001 From: Vladimir Svyatski Date: Wed, 27 Mar 2019 01:54:54 +0200 Subject: [PATCH 02/19] Make KeeShare user messages easier to understand (#2824) --- src/keeshare/KeeShare.cpp | 16 +++++----- src/keeshare/ShareObserver.cpp | 2 +- .../group/EditGroupWidgetKeeShare.cpp | 31 +++++++++++-------- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/keeshare/KeeShare.cpp b/src/keeshare/KeeShare.cpp index 08c7b4f1..01d11f02 100644 --- a/src/keeshare/KeeShare.cpp +++ b/src/keeshare/KeeShare.cpp @@ -164,13 +164,13 @@ QString KeeShare::sharingLabel(const Group* group) const auto reference = referenceOf(share); switch (reference.type) { case KeeShareSettings::Inactive: - return tr("Disabled share %1").arg(reference.path); + return tr("Inactive share %1").arg(reference.path); case KeeShareSettings::ImportFrom: - return tr("Import from share %1").arg(reference.path); + return tr("Imported from %1").arg(reference.path); case KeeShareSettings::ExportTo: - return tr("Export to share %1").arg(reference.path); + return tr("Exported to %1").arg(reference.path); case KeeShareSettings::SynchronizeWith: - return tr("Synchronize with share %1").arg(reference.path); + return tr("Synchronized with %1").arg(reference.path); } return {}; @@ -196,13 +196,13 @@ QString KeeShare::referenceTypeLabel(const KeeShareSettings::Reference& referenc { switch (reference.type) { case KeeShareSettings::Inactive: - return tr("Disabled share"); + return tr("Inactive share"); case KeeShareSettings::ImportFrom: - return tr("Import from"); + return tr("Imported from"); case KeeShareSettings::ExportTo: - return tr("Export to"); + return tr("Exported to"); case KeeShareSettings::SynchronizeWith: - return tr("Synchronize with"); + return tr("Synchronized with"); } return ""; } diff --git a/src/keeshare/ShareObserver.cpp b/src/keeshare/ShareObserver.cpp index 63d8358c..82d02a7d 100644 --- a/src/keeshare/ShareObserver.cpp +++ b/src/keeshare/ShareObserver.cpp @@ -84,7 +84,7 @@ namespace key.openKey(QString()); const auto signer = Signature(); if (!signer.verify(data, sign.signature, key)) { - qCritical("Invalid signature for sharing container %s.", qPrintable(reference.path)); + qCritical("Invalid signature for shared container %s.", qPrintable(reference.path)); return {Invalid, KeeShareSettings::Certificate()}; } diff --git a/src/keeshare/group/EditGroupWidgetKeeShare.cpp b/src/keeshare/group/EditGroupWidgetKeeShare.cpp index 49e64063..0170a82a 100644 --- a/src/keeshare/group/EditGroupWidgetKeeShare.cpp +++ b/src/keeshare/group/EditGroupWidgetKeeShare.cpp @@ -68,13 +68,13 @@ EditGroupWidgetKeeShare::EditGroupWidgetKeeShare(QWidget* parent) name = tr("Inactive"); break; case KeeShareSettings::ImportFrom: - name = tr("Import from path"); + name = tr("Import"); break; case KeeShareSettings::ExportTo: - name = tr("Export to path"); + name = tr("Export"); break; case KeeShareSettings::SynchronizeWith: - name = tr("Synchronize with path"); + name = tr("Synchronize"); break; } m_ui->typeComboBox->insertItem(type, name, static_cast(type)); @@ -124,10 +124,10 @@ void EditGroupWidgetKeeShare::showSharingState() } } if (!supported) { - m_ui->messageWidget->showMessage( - tr("Your KeePassXC version does not support sharing your container type. Please use %1.") - .arg(supportedExtensions.join(", ")), - MessageWidget::Warning); + m_ui->messageWidget->showMessage(tr("Your KeePassXC version does not support sharing this container type.\n" + "Supported extensions are: %1.") + .arg(supportedExtensions.join(", ")), + MessageWidget::Warning); return; } @@ -149,18 +149,18 @@ void EditGroupWidgetKeeShare::showSharingState() (other.isImporting() && reference.isExporting()) || (other.isExporting() && reference.isImporting()); } if (conflictExport) { - m_ui->messageWidget->showMessage(tr("The export container %1 is already referenced.").arg(reference.path), + m_ui->messageWidget->showMessage(tr("%1 is already being exported by this database.").arg(reference.path), MessageWidget::Error); return; } if (multipleImport) { - m_ui->messageWidget->showMessage(tr("The import container %1 is already imported.").arg(reference.path), + m_ui->messageWidget->showMessage(tr("%1 is already being imported by this database.").arg(reference.path), MessageWidget::Warning); return; } if (cycleImportExport) { m_ui->messageWidget->showMessage( - tr("The container %1 imported and export by different groups.").arg(reference.path), + tr("%1 is being imported and exported by different groups in this database.").arg(reference.path), MessageWidget::Warning); return; } @@ -169,15 +169,20 @@ void EditGroupWidgetKeeShare::showSharingState() } const auto active = KeeShare::active(); if (!active.in && !active.out) { - m_ui->messageWidget->showMessage(tr("Database sharing is disabled"), MessageWidget::Information); + m_ui->messageWidget->showMessage( + tr("KeeShare is currently disabled. You can enable import/export in the application settings.", + "KeeShare is a proper noun"), + MessageWidget::Information); return; } if (active.in && !active.out) { - m_ui->messageWidget->showMessage(tr("Database export is disabled"), MessageWidget::Information); + m_ui->messageWidget->showMessage(tr("Database export is currently disabled by application settings."), + MessageWidget::Information); return; } if (!active.in && active.out) { - m_ui->messageWidget->showMessage(tr("Database import is disabled"), MessageWidget::Information); + m_ui->messageWidget->showMessage(tr("Database import is currently disabled by application settings."), + MessageWidget::Information); return; } } From 52d411f423391ccb3b952a08afc1f6465054245b Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Tue, 26 Mar 2019 17:14:42 -0400 Subject: [PATCH 03/19] Use existing base64 check in Tools namespace --- src/format/KdbxXmlReader.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/format/KdbxXmlReader.cpp b/src/format/KdbxXmlReader.cpp index 84d597bd..ab2b9aeb 100644 --- a/src/format/KdbxXmlReader.cpp +++ b/src/format/KdbxXmlReader.cpp @@ -1028,10 +1028,8 @@ bool KdbxXmlReader::readBool() QDateTime KdbxXmlReader::readDateTime() { - static QRegularExpression b64regex("^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$"); QString str = readString(); - - if (b64regex.match(str).hasMatch()) { + if (Tools::isBase64(str.toLatin1())) { QByteArray secsBytes = QByteArray::fromBase64(str.toUtf8()).leftJustified(8, '\0', true).left(8); qint64 secs = Endian::bytesToSizedInt(secsBytes, KeePass2::BYTEORDER); return QDateTime(QDate(1, 1, 1), QTime(0, 0, 0, 0), Qt::UTC).addSecs(secs); From edef225eab4698ea0a0bc6bc7996369ff5d1c6fb Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Tue, 26 Mar 2019 22:23:16 -0400 Subject: [PATCH 04/19] Fix opening files from command line * Fix #2877 - password is unchecked by default * Smarter activation of key components based on contents of text entry fields * Prevent multiple copies of the same database from opening when the canonicalFileName != fileName --- src/core/Database.cpp | 20 ++++++++++++++++++++ src/core/Database.h | 1 + src/gui/DatabaseOpenWidget.cpp | 23 +++++++++++------------ src/gui/DatabaseTabWidget.cpp | 10 +++------- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 22484cb8..cb3039cd 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -388,11 +388,31 @@ const Metadata* Database::metadata() const return m_metadata; } +/** + * Returns the original file path that was provided for + * this database. This path may not exist, may contain + * unresolved symlinks, or have malformed slashes. + * + * @return original file path + */ QString Database::filePath() const { return m_data.filePath; } +/** + * Returns the canonical file path of this databases' + * set file path. This returns an empty string if the + * file does not exist or cannot be resolved. + * + * @return canonical file path + */ +QString Database::canonicalFilePath() const +{ + QFileInfo fileInfo(m_data.filePath); + return fileInfo.canonicalFilePath(); +} + void Database::setFilePath(const QString& filePath) { if (filePath == m_data.filePath) { diff --git a/src/core/Database.h b/src/core/Database.h index 8df2b931..bfdbf791 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -82,6 +82,7 @@ public: QUuid uuid() const; QString filePath() const; + QString canonicalFilePath() const; void setFilePath(const QString& filePath); Metadata* metadata(); diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 15584664..c7237a36 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -45,7 +45,6 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) m_ui->setupUi(this); m_ui->messageWidget->setHidden(true); - m_ui->checkPassword->setChecked(true); QFont font = m_ui->labelHeadline->font(); font.setBold(true); @@ -159,7 +158,7 @@ void DatabaseOpenWidget::clearForms() m_ui->editPassword->setText(""); m_ui->comboKeyFile->clear(); m_ui->comboKeyFile->setEditText(""); - m_ui->checkPassword->setChecked(true); + m_ui->checkPassword->setChecked(false); m_ui->checkKeyFile->setChecked(false); m_ui->checkChallengeResponse->setChecked(false); m_ui->checkTouchID->setChecked(false); @@ -174,13 +173,8 @@ QSharedPointer DatabaseOpenWidget::database() void DatabaseOpenWidget::enterKey(const QString& pw, const QString& keyFile) { - if (!pw.isEmpty()) { - m_ui->editPassword->setText(pw); - } - if (!keyFile.isEmpty()) { - m_ui->comboKeyFile->setEditText(keyFile); - } - + m_ui->editPassword->setText(pw); + m_ui->comboKeyFile->setEditText(keyFile); openDatabase(); } @@ -339,17 +333,20 @@ void DatabaseOpenWidget::reject() void DatabaseOpenWidget::activatePassword() { - m_ui->checkPassword->setChecked(true); + bool hasPassword = !m_ui->editPassword->text().isEmpty(); + m_ui->checkPassword->setChecked(hasPassword); } void DatabaseOpenWidget::activateKeyFile() { - m_ui->checkKeyFile->setChecked(true); + bool hasKeyFile = !m_ui->comboKeyFile->lineEdit()->text().isEmpty(); + m_ui->checkKeyFile->setChecked(hasKeyFile); } void DatabaseOpenWidget::activateChallengeResponse() { - m_ui->checkChallengeResponse->setChecked(true); + bool hasCR = m_ui->comboChallengeResponse->currentData().toInt() != -1; + m_ui->checkChallengeResponse->setChecked(hasCR); } void DatabaseOpenWidget::browseKeyFile() @@ -372,6 +369,7 @@ void DatabaseOpenWidget::pollYubikey() m_ui->checkChallengeResponse->setChecked(false); m_ui->comboChallengeResponse->setEnabled(false); m_ui->comboChallengeResponse->clear(); + m_ui->comboChallengeResponse->addItem(tr("-- SELECT --"), -1); m_ui->yubikeyProgress->setVisible(true); // YubiKey init is slow, detect asynchronously to not block the UI @@ -388,6 +386,7 @@ void DatabaseOpenWidget::yubikeyDetected(int slot, bool blocking) QHash lastChallengeResponse = config()->get("LastChallengeResponse").toHash(); if (lastChallengeResponse.contains(m_filename)) { m_ui->checkChallengeResponse->setChecked(true); + m_ui->comboChallengeResponse->setCurrentIndex(1); } } } diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index 7693c901..a7fed628 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -157,10 +157,8 @@ void DatabaseTabWidget::addDatabaseTab(const QString& filePath, for (int i = 0, c = count(); i < c; ++i) { auto* dbWidget = databaseWidgetFromIndex(i); Q_ASSERT(dbWidget); - if (dbWidget && dbWidget->database()->filePath() == canonicalFilePath) { - if (!password.isEmpty()) { - dbWidget->performUnlockDatabase(password, keyfile); - } + if (dbWidget && dbWidget->database()->canonicalFilePath() == canonicalFilePath) { + dbWidget->performUnlockDatabase(password, keyfile); if (!inBackground) { // switch to existing tab if file is already open setCurrentIndex(indexOf(dbWidget)); @@ -171,9 +169,7 @@ void DatabaseTabWidget::addDatabaseTab(const QString& filePath, auto* dbWidget = new DatabaseWidget(QSharedPointer::create(filePath), this); addDatabaseTab(dbWidget, inBackground); - if (!password.isEmpty()) { - dbWidget->performUnlockDatabase(password, keyfile); - } + dbWidget->performUnlockDatabase(password, keyfile); updateLastDatabases(filePath); } From cb2900f5a98b3774b9dfaa4c6ead4f1a3b18c6fc Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sat, 30 Mar 2019 21:31:32 -0400 Subject: [PATCH 05/19] Fix database master key dirtying * When removing portions of the master key, the key is marked dirty for saving * Properly clear password and other fields in edit entry widget and password widgets --- src/gui/dbsettings/DatabaseSettingsWidgetMasterKey.cpp | 7 +++---- src/gui/entry/EditEntryWidget.cpp | 7 +++++++ src/gui/masterkey/PasswordEditWidget.cpp | 9 +++++++++ src/gui/masterkey/PasswordEditWidget.h | 1 + 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/gui/dbsettings/DatabaseSettingsWidgetMasterKey.cpp b/src/gui/dbsettings/DatabaseSettingsWidgetMasterKey.cpp index d1a64b52..a8425aae 100644 --- a/src/gui/dbsettings/DatabaseSettingsWidgetMasterKey.cpp +++ b/src/gui/dbsettings/DatabaseSettingsWidgetMasterKey.cpp @@ -77,11 +77,8 @@ void DatabaseSettingsWidgetMasterKey::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); - m_isDirty = true; - return; } - bool isDirty = false; bool hasAdditionalKeys = false; for (const auto& key : m_db->key()->keys()) { if (key->uuid() == PasswordKey::UUID) { @@ -103,7 +100,9 @@ void DatabaseSettingsWidgetMasterKey::load(QSharedPointer db) setAdditionalKeyOptionsVisible(hasAdditionalKeys); - m_isDirty = isDirty; + connect(m_passwordEditWidget->findChild("removeButton"), SIGNAL(clicked()), SLOT(markDirty())); + connect(m_keyFileEditWidget->findChild("removeButton"), SIGNAL(clicked()), SLOT(markDirty())); + connect(m_yubiKeyEditWidget->findChild("removeButton"), SIGNAL(clicked()), SLOT(markDirty())); } void DatabaseSettingsWidgetMasterKey::initialize() diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index 9a4c1600..74a1fcd3 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -993,6 +993,13 @@ void EditEntryWidget::clear() { m_entry = nullptr; m_db.reset(); + + m_mainUi->titleEdit->setText(""); + m_mainUi->passwordEdit->setText(""); + m_mainUi->passwordRepeatEdit->setText(""); + m_mainUi->urlEdit->setText(""); + m_mainUi->notesEdit->clear(); + m_entryAttributes->clear(); m_advancedUi->attachmentsWidget->clearAttachments(); m_autoTypeAssoc->clear(); diff --git a/src/gui/masterkey/PasswordEditWidget.cpp b/src/gui/masterkey/PasswordEditWidget.cpp index 86d629da..6f6cf4d9 100644 --- a/src/gui/masterkey/PasswordEditWidget.cpp +++ b/src/gui/masterkey/PasswordEditWidget.cpp @@ -92,6 +92,15 @@ void PasswordEditWidget::initComponentEditWidget(QWidget* widget) m_compUi->enterPasswordEdit->setFocus(); } +void PasswordEditWidget::hideEvent(QHideEvent* event) +{ + if (!isVisible() && m_compUi->enterPasswordEdit) { + m_compUi->enterPasswordEdit->setText(""); + } + + QWidget::hideEvent(event); +} + bool PasswordEditWidget::validate(QString& errorMessage) const { if (m_compUi->enterPasswordEdit->text() != m_compUi->repeatPasswordEdit->text()) { diff --git a/src/gui/masterkey/PasswordEditWidget.h b/src/gui/masterkey/PasswordEditWidget.h index 9f3eb75c..57c225c1 100644 --- a/src/gui/masterkey/PasswordEditWidget.h +++ b/src/gui/masterkey/PasswordEditWidget.h @@ -44,6 +44,7 @@ public: protected: QWidget* componentEditWidget() override; void initComponentEditWidget(QWidget* widget) override; + void hideEvent(QHideEvent* event) override; private slots: void showPasswordGenerator(); From f49a8a7f70fd5cdf0da6ed555c651bfcc2cbe8ca Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Tue, 2 Apr 2019 00:01:58 +0200 Subject: [PATCH 06/19] Fix key component widget initialization and password field echo mode on database open --- src/gui/DatabaseOpenWidget.cpp | 6 ++---- src/gui/masterkey/KeyComponentWidget.cpp | 20 ++++++++++++++------ src/gui/masterkey/KeyComponentWidget.h | 5 ++++- src/gui/masterkey/PasswordEditWidget.cpp | 3 +++ 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index c7237a36..0f540bca 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -185,9 +185,7 @@ void DatabaseOpenWidget::openDatabase() return; } - if (!m_ui->editPassword->isPasswordVisible()) { - m_ui->editPassword->setShowPassword(false); - } + m_ui->editPassword->setShowPassword(false); QCoreApplication::processEvents(); m_db.reset(new Database()); @@ -369,7 +367,7 @@ void DatabaseOpenWidget::pollYubikey() m_ui->checkChallengeResponse->setChecked(false); m_ui->comboChallengeResponse->setEnabled(false); m_ui->comboChallengeResponse->clear(); - m_ui->comboChallengeResponse->addItem(tr("-- SELECT --"), -1); + m_ui->comboChallengeResponse->addItem(tr("Select slot..."), -1); m_ui->yubikeyProgress->setVisible(true); // YubiKey init is slow, detect asynchronously to not block the UI diff --git a/src/gui/masterkey/KeyComponentWidget.cpp b/src/gui/masterkey/KeyComponentWidget.cpp index 7d795aca..da362a95 100644 --- a/src/gui/masterkey/KeyComponentWidget.cpp +++ b/src/gui/masterkey/KeyComponentWidget.cpp @@ -37,7 +37,7 @@ KeyComponentWidget::KeyComponentWidget(const QString& name, QWidget* parent) connect(m_ui->removeButton, SIGNAL(clicked(bool)), SIGNAL(componentRemovalRequested())); connect(m_ui->cancelButton, SIGNAL(clicked(bool)), SLOT(cancelEdit())); - connect(m_ui->stackedWidget, SIGNAL(currentChanged(int)), SLOT(reset())); + connect(m_ui->stackedWidget, SIGNAL(currentChanged(int)), SLOT(resetComponentEditWidget())); connect(this, SIGNAL(nameChanged(QString)), SLOT(updateComponentName(QString))); connect(this, SIGNAL(descriptionChanged(QString)), SLOT(updateComponentDescription(QString))); @@ -46,11 +46,13 @@ KeyComponentWidget::KeyComponentWidget(const QString& name, QWidget* parent) connect(this, SIGNAL(componentRemovalRequested()), SLOT(doRemove())); connect(this, SIGNAL(componentAddChanged(bool)), SLOT(updateAddStatus(bool))); - blockSignals(true); + bool prev = blockSignals(true); setComponentName(name); + blockSignals(prev); + + prev = m_ui->stackedWidget->blockSignals(true); m_ui->stackedWidget->setCurrentIndex(Page::AddNew); - updateSize(); - blockSignals(false); + m_ui->stackedWidget->blockSignals(prev); } KeyComponentWidget::~KeyComponentWidget() @@ -164,9 +166,15 @@ void KeyComponentWidget::cancelEdit() emit editCanceled(); } -void KeyComponentWidget::reset() +void KeyComponentWidget::showEvent(QShowEvent* event) { - if (static_cast(m_ui->stackedWidget->currentIndex()) == Page::Edit) { + QWidget::showEvent(event); + resetComponentEditWidget(); +} + +void KeyComponentWidget::resetComponentEditWidget() +{ + if (m_ui->componentWidgetLayout->isEmpty() || static_cast(m_ui->stackedWidget->currentIndex()) == Page::Edit) { if (!m_ui->componentWidgetLayout->isEmpty()) { auto* item = m_ui->componentWidgetLayout->takeAt(0); if (item->widget()) { diff --git a/src/gui/masterkey/KeyComponentWidget.h b/src/gui/masterkey/KeyComponentWidget.h index cf2ae494..f184c132 100644 --- a/src/gui/masterkey/KeyComponentWidget.h +++ b/src/gui/masterkey/KeyComponentWidget.h @@ -109,6 +109,9 @@ signals: void editCanceled(); void componentRemovalRequested(); +protected: + void showEvent(QShowEvent* event) override ; + private slots: void updateComponentName(const QString& name); void updateComponentDescription(const QString& decription); @@ -117,7 +120,7 @@ private slots: void doEdit(); void doRemove(); void cancelEdit(); - void reset(); + void resetComponentEditWidget(); void updateSize(); private: diff --git a/src/gui/masterkey/PasswordEditWidget.cpp b/src/gui/masterkey/PasswordEditWidget.cpp index 6f6cf4d9..de00199b 100644 --- a/src/gui/masterkey/PasswordEditWidget.cpp +++ b/src/gui/masterkey/PasswordEditWidget.cpp @@ -94,8 +94,11 @@ void PasswordEditWidget::initComponentEditWidget(QWidget* widget) void PasswordEditWidget::hideEvent(QHideEvent* event) { + Q_ASSERT(m_compUi->enterPasswordEdit); + if (!isVisible() && m_compUi->enterPasswordEdit) { m_compUi->enterPasswordEdit->setText(""); + m_compUi->repeatPasswordEdit->setText(""); } QWidget::hideEvent(event); From e025444c869b40aec75fe90e095cd888f64d1b21 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Tue, 2 Apr 2019 22:32:48 +0200 Subject: [PATCH 07/19] Fix double password edit field --- src/gui/masterkey/KeyComponentWidget.cpp | 19 +++++++------------ src/gui/masterkey/KeyComponentWidget.h | 2 ++ 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/gui/masterkey/KeyComponentWidget.cpp b/src/gui/masterkey/KeyComponentWidget.cpp index da362a95..769cbab9 100644 --- a/src/gui/masterkey/KeyComponentWidget.cpp +++ b/src/gui/masterkey/KeyComponentWidget.cpp @@ -168,25 +168,20 @@ void KeyComponentWidget::cancelEdit() void KeyComponentWidget::showEvent(QShowEvent* event) { - QWidget::showEvent(event); resetComponentEditWidget(); + QWidget::showEvent(event); } void KeyComponentWidget::resetComponentEditWidget() { - if (m_ui->componentWidgetLayout->isEmpty() || static_cast(m_ui->stackedWidget->currentIndex()) == Page::Edit) { - if (!m_ui->componentWidgetLayout->isEmpty()) { - auto* item = m_ui->componentWidgetLayout->takeAt(0); - if (item->widget()) { - delete item->widget(); - } - delete item; + if (!m_componentWidget || static_cast(m_ui->stackedWidget->currentIndex()) == Page::Edit) { + if (m_componentWidget) { + delete m_componentWidget; } - QWidget* widget = componentEditWidget(); - m_ui->componentWidgetLayout->addWidget(widget); - - initComponentEditWidget(widget); + m_componentWidget = componentEditWidget(); + m_ui->componentWidgetLayout->addWidget(m_componentWidget); + initComponentEditWidget(m_componentWidget); } QTimer::singleShot(0, this, SLOT(updateSize())); diff --git a/src/gui/masterkey/KeyComponentWidget.h b/src/gui/masterkey/KeyComponentWidget.h index f184c132..63079863 100644 --- a/src/gui/masterkey/KeyComponentWidget.h +++ b/src/gui/masterkey/KeyComponentWidget.h @@ -20,6 +20,7 @@ #include #include +#include namespace Ui { @@ -128,6 +129,7 @@ private: Page m_previousPage = Page::AddNew; QString m_componentName; QString m_componentDescription; + QPointer m_componentWidget; const QScopedPointer m_ui; }; From ec829315730e07e5db42358cfa36a190eaea55c4 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Tue, 26 Mar 2019 18:49:26 -0400 Subject: [PATCH 08/19] Fix broken safe saves across file systems * Fix #2888 * Qt has an undocumented rename implementation for QTemporaryFile that does not fallback to the copy implementation. Forcing the use of QFile::rename(...) allows for this fallback and protects against cross-device link errors. --- src/core/Database.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index cb3039cd..3593466e 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -241,20 +241,16 @@ bool Database::save(const QString& filePath, QString* error, bool atomic, bool b // Delete the original db and move the temp file in place QFile::remove(filePath); -#ifdef Q_OS_LINUX - // workaround to make this workaround work, see: https://bugreports.qt.io/browse/QTBUG-64008 - if (tempFile.copy(filePath)) { - // successfully saved database file - return true; - } -#else - if (tempFile.rename(filePath)) { + + // Note: call into the QFile rename instead of QTemporaryFile + // due to an undocumented difference in how the function handles + // errors. This prevents errors when saving across file systems. + if (tempFile.QFile::rename(filePath)) { // successfully saved database file tempFile.setAutoRemove(false); setFilePath(filePath); return true; } -#endif } if (error) { *error = tempFile.errorString(); From 3b0b5d85e962b57b14507f9b07ee187aa35ecaf4 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Wed, 3 Apr 2019 10:23:18 -0400 Subject: [PATCH 09/19] Correct behaviors when saving database fails * Mark database dirty if saving fails * Restore database file from backup if unsafe save fails between deleting database file and copying temporary file into place * Improve error message display for opening and saving database files * Do not automatically retry saving after failure. This prevents deletion of the backup database file and improves user awareness of issues. --- src/core/Database.cpp | 28 +++++++++++++++++++++++++++- src/core/Database.h | 1 + src/gui/DatabaseTabWidget.cpp | 3 ++- src/gui/DatabaseWidget.cpp | 24 +++++++++++++++--------- src/gui/DatabaseWidget.h | 4 +++- src/gui/MessageWidget.cpp | 1 + src/gui/MessageWidget.h | 1 + 7 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 3593466e..8c1f2213 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -222,6 +222,7 @@ bool Database::save(const QString& filePath, QString* error, bool atomic, bool b return true; } } + if (error) { *error = saveFile.errorString(); } @@ -246,18 +247,25 @@ bool Database::save(const QString& filePath, QString* error, bool atomic, bool b // due to an undocumented difference in how the function handles // errors. This prevents errors when saving across file systems. if (tempFile.QFile::rename(filePath)) { - // successfully saved database file + // successfully saved the database tempFile.setAutoRemove(false); setFilePath(filePath); return true; + } else { + // restore the database from the backup + if (backup) { + restoreDatabase(filePath); + } } } + if (error) { *error = tempFile.errorString(); } } // Saving failed + markAsModified(); return false; } @@ -316,6 +324,24 @@ bool Database::backupDatabase(const QString& filePath) return QFile::copy(filePath, backupFilePath); } +/** + * Restores the database file from the backup file with + * name .old. to filePath. This will + * overwrite the existing file! + * + * @param filePath Path to the file to restore + * @return true on success + */ +bool Database::restoreDatabase(const QString& filePath) +{ + static auto re = QRegularExpression("^(.*?)(\\.[^.]+)?$"); + + auto match = re.match(filePath); + auto backupFilePath = match.captured(1) + ".old" + match.captured(2); + QFile::remove(filePath); + return QFile::copy(backupFilePath, filePath); +} + bool Database::isReadOnly() const { return m_data.isReadOnly; diff --git a/src/core/Database.h b/src/core/Database.h index bfdbf791..27bb3e4a 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -172,6 +172,7 @@ private: bool writeDatabase(QIODevice* device, QString* error = nullptr); bool backupDatabase(const QString& filePath); + bool restoreDatabase(const QString& filePath); Metadata* const m_metadata; DatabaseData m_data; diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index a7fed628..313bfabb 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -150,7 +150,8 @@ void DatabaseTabWidget::addDatabaseTab(const QString& filePath, QFileInfo fileInfo(filePath); QString canonicalFilePath = fileInfo.canonicalFilePath(); if (canonicalFilePath.isEmpty()) { - emit messageGlobal(tr("The database file does not exist or is not accessible."), MessageWidget::Error); + emit messageGlobal(tr("Failed to open %1. It either does not exist or is not accessible.").arg(filePath), + MessageWidget::Error); return; } diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index e4196734..abf8fcf7 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -89,6 +89,7 @@ DatabaseWidget::DatabaseWidget(QSharedPointer db, QWidget* parent) , m_databaseOpenWidget(new DatabaseOpenWidget(this)) , m_keepass1OpenWidget(new KeePass1OpenWidget(this)) , m_groupView(new GroupView(m_db.data(), m_mainSplitter)) + , m_saveAttempts(0) , m_fileWatcher(new DelayingFileWatcher(this)) { m_messageWidget->setHidden(true); @@ -859,6 +860,7 @@ void DatabaseWidget::loadDatabase(bool accepted) replaceDatabase(openWidget->database()); switchToMainView(); m_fileWatcher->restart(); + m_saveAttempts = 0; emit databaseUnlocked(); } else { m_fileWatcher->stop(); @@ -1512,7 +1514,7 @@ EntryView* DatabaseWidget::entryView() * @param attempt current save attempt or -1 to disable attempts * @return true on success */ -bool DatabaseWidget::save(int attempt) +bool DatabaseWidget::save() { // Never allow saving a locked database; it causes corruption Q_ASSERT(!isLocked()); @@ -1527,6 +1529,8 @@ bool DatabaseWidget::save(int attempt) } blockAutoReload(true); + ++m_saveAttempts; + // TODO: Make this async, but lock out the database widget to prevent re-entrance bool useAtomicSaves = config()->get("UseAtomicSaves", true).toBool(); QString errorMessage; @@ -1534,14 +1538,11 @@ bool DatabaseWidget::save(int attempt) blockAutoReload(false); if (ok) { + m_saveAttempts = 0; return true; } - if (attempt >= 0 && attempt <= 2) { - return save(attempt + 1); - } - - if (attempt > 2 && useAtomicSaves) { + if (m_saveAttempts > 2 && useAtomicSaves) { // Saving failed 3 times, issue a warning and attempt to resolve auto result = MessageBox::question(this, tr("Disable safe saves?"), @@ -1552,11 +1553,15 @@ bool DatabaseWidget::save(int attempt) MessageBox::Disable); if (result == MessageBox::Disable) { config()->set("UseAtomicSaves", false); - return save(attempt + 1); + return save(); } } - showMessage(tr("Writing the database failed.\n%1").arg(errorMessage), MessageWidget::Error); + showMessage(tr("Writing the database failed: %1").arg(errorMessage), + MessageWidget::Error, + true, + MessageWidget::LongAutoHideTimeout); + return false; } @@ -1585,8 +1590,9 @@ bool DatabaseWidget::saveAs() // Ensure we don't recurse back into this function m_db->setReadOnly(false); m_db->setFilePath(newFilePath); + m_saveAttempts = 0; - if (!save(-1)) { + if (!save()) { // Failed to save, try again continue; } diff --git a/src/gui/DatabaseWidget.h b/src/gui/DatabaseWidget.h index 9c278899..11b2f710 100644 --- a/src/gui/DatabaseWidget.h +++ b/src/gui/DatabaseWidget.h @@ -144,7 +144,7 @@ signals: public slots: bool lock(); - bool save(int attempt = 0); + bool save(); bool saveAs(); void replaceDatabase(QSharedPointer db); @@ -255,6 +255,8 @@ private: QUuid m_groupBeforeLock; QUuid m_entryBeforeLock; + int m_saveAttempts; + // Search state EntrySearcher* m_EntrySearcher; QString m_lastSearchText; diff --git a/src/gui/MessageWidget.cpp b/src/gui/MessageWidget.cpp index 5b18a583..4b7e67a2 100644 --- a/src/gui/MessageWidget.cpp +++ b/src/gui/MessageWidget.cpp @@ -23,6 +23,7 @@ #include const int MessageWidget::DefaultAutoHideTimeout = 6000; +const int MessageWidget::LongAutoHideTimeout = 15000; const int MessageWidget::DisableAutoHide = -1; MessageWidget::MessageWidget(QWidget* parent) diff --git a/src/gui/MessageWidget.h b/src/gui/MessageWidget.h index eac50601..fe4baec4 100644 --- a/src/gui/MessageWidget.h +++ b/src/gui/MessageWidget.h @@ -33,6 +33,7 @@ public: int autoHideTimeout() const; static const int DefaultAutoHideTimeout; + static const int LongAutoHideTimeout; static const int DisableAutoHide; signals: From 791b796c234176646f43ce61da98fb1ec92e54f0 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sun, 7 Apr 2019 09:21:58 -0400 Subject: [PATCH 10/19] Additional layer of protection for unsafe saves * Attempt to restore database, if that fails retain the temporary file and tell the user where it is located --- src/core/Database.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 8c1f2213..ee888327 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -251,11 +251,15 @@ bool Database::save(const QString& filePath, QString* error, bool atomic, bool b tempFile.setAutoRemove(false); setFilePath(filePath); return true; - } else { - // restore the database from the backup - if (backup) { - restoreDatabase(filePath); + } else if (!backup || !restoreDatabase(filePath)) { + // Failed to copy new database in place, and + // failed to restore from backup or backups disabled + tempFile.setAutoRemove(false); + if (error) { + *error = tr("%1\nBackup database located at %2").arg(tempFile.errorString(), tempFile.fileName()); } + markAsModified(); + return false; } } @@ -338,8 +342,12 @@ bool Database::restoreDatabase(const QString& filePath) auto match = re.match(filePath); auto backupFilePath = match.captured(1) + ".old" + match.captured(2); - QFile::remove(filePath); - return QFile::copy(backupFilePath, filePath); + // Only try to restore if the backup file actually exists + if (QFile::exists(backupFilePath)) { + QFile::remove(filePath); + return QFile::copy(backupFilePath, filePath); + } + return false; } bool Database::isReadOnly() const From 0201fcd400cbc89f1231ea685d10a0d847221969 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Fri, 5 Apr 2019 08:31:37 -0400 Subject: [PATCH 11/19] Improved error messages when opening database * Reduced wording and confusion * Streamlined delivery format * Fix #813 --- src/format/Kdbx3Reader.cpp | 3 ++- src/format/Kdbx4Reader.cpp | 3 ++- src/format/KeePass1Reader.cpp | 3 ++- src/gui/DatabaseOpenWidget.cpp | 7 +++---- tests/TestCli.cpp | 6 ++---- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/format/Kdbx3Reader.cpp b/src/format/Kdbx3Reader.cpp index 4fec7471..9196bc61 100644 --- a/src/format/Kdbx3Reader.cpp +++ b/src/format/Kdbx3Reader.cpp @@ -78,7 +78,8 @@ bool Kdbx3Reader::readDatabaseImpl(QIODevice* device, QByteArray realStart = cipherStream.read(32); if (realStart != m_streamStartBytes) { - raiseError(tr("Wrong key or database file is corrupt.")); + raiseError(tr("Invalid credentials were provided, please try again.\n" + "If this reoccurs, then your database file may be corrupt.")); return false; } diff --git a/src/format/Kdbx4Reader.cpp b/src/format/Kdbx4Reader.cpp index fbdf865b..4bb0202b 100644 --- a/src/format/Kdbx4Reader.cpp +++ b/src/format/Kdbx4Reader.cpp @@ -71,7 +71,8 @@ bool Kdbx4Reader::readDatabaseImpl(QIODevice* device, // clang-format off QByteArray hmacKey = KeePass2::hmacKey(m_masterSeed, db->transformedMasterKey()); if (headerHmac != CryptoHash::hmac(headerData, HmacBlockStream::getHmacKey(UINT64_MAX, hmacKey), CryptoHash::Sha256)) { - raiseError(tr("Wrong key or database file is corrupt. (HMAC mismatch)")); + raiseError(tr("Invalid credentials were provided, please try again.\n" + "If this reoccurs, then your database file may be corrupt.") + " " + tr("(HMAC mismatch)")); return false; } HmacBlockStream hmacStream(device, hmacKey); diff --git a/src/format/KeePass1Reader.cpp b/src/format/KeePass1Reader.cpp index e4244935..0319b1b2 100644 --- a/src/format/KeePass1Reader.cpp +++ b/src/format/KeePass1Reader.cpp @@ -372,7 +372,8 @@ KeePass1Reader::testKeys(const QString& password, const QByteArray& keyfileData, } if (!cipherStream) { - raiseError(tr("Wrong key or database file is corrupt.")); + raiseError(tr("Invalid credentials were provided, please try again.\n" + "If this reoccurs, then your database file may be corrupt.")); } return cipherStream.take(); diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 0f540bca..ced72485 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -194,8 +194,7 @@ void DatabaseOpenWidget::openDatabase() bool ok = m_db->open(m_filename, masterKey, &error, false); QApplication::restoreOverrideCursor(); if (!ok) { - m_ui->messageWidget->showMessage(tr("Unable to open the database:\n%1").arg(error), - MessageWidget::MessageType::Error); + m_ui->messageWidget->showMessage(error, MessageWidget::MessageType::Error); return; } @@ -223,7 +222,7 @@ void DatabaseOpenWidget::openDatabase() } emit dialogFinished(true); } else { - m_ui->messageWidget->showMessage(tr("Unable to open the database:\n%1").arg(error), MessageWidget::Error); + m_ui->messageWidget->showMessage(error, MessageWidget::Error); m_ui->editPassword->setText(""); #ifdef WITH_XC_TOUCHID @@ -268,7 +267,7 @@ QSharedPointer DatabaseOpenWidget::databaseKey() QString keyFilename = m_ui->comboKeyFile->currentText(); QString errorMsg; if (!key->load(keyFilename, &errorMsg)) { - m_ui->messageWidget->showMessage(tr("Can't open key file:\n%1").arg(errorMsg), MessageWidget::Error); + m_ui->messageWidget->showMessage(tr("Failed to open key file: %1").arg(errorMsg), MessageWidget::Error); return {}; } if (key->type() != FileKey::Hashed && !config()->get("Messages/NoLegacyKeyFileWarning").toBool()) { diff --git a/tests/TestCli.cpp b/tests/TestCli.cpp index 3ba40b90..9574f6d3 100644 --- a/tests/TestCli.cpp +++ b/tests/TestCli.cpp @@ -702,8 +702,7 @@ void TestCli::testKeyFileOption() m_stdoutFile->readLine(); // skip password prompt m_stderrFile->seek(posErr); QCOMPARE(m_stdoutFile->readAll(), QByteArray("")); - QCOMPARE(m_stderrFile->readAll(), - QByteArray("Error while reading the database: Wrong key or database file is corrupt. (HMAC mismatch)\n")); + QVERIFY(m_stderrFile->readAll().contains("Invalid credentials were provided")); // Should raise an error if key file path is invalid. pos = m_stdoutFile->pos(); @@ -736,8 +735,7 @@ void TestCli::testNoPasswordOption() m_stdoutFile->readLine(); // skip password prompt m_stderrFile->seek(posErr); QCOMPARE(m_stdoutFile->readAll(), QByteArray("")); - QCOMPARE(m_stderrFile->readAll(), - QByteArray("Error while reading the database: Wrong key or database file is corrupt. (HMAC mismatch)\n")); + QVERIFY(m_stderrFile->readAll().contains("Invalid credentials were provided")); } void TestCli::testList() From 88c8cdd800413fd72b320c6780366cf6db0da589 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Fri, 5 Apr 2019 08:42:35 -0400 Subject: [PATCH 12/19] Add note to restart after changing language * Fix #2713 --- src/gui/ApplicationSettingsWidgetGeneral.ui | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/gui/ApplicationSettingsWidgetGeneral.ui b/src/gui/ApplicationSettingsWidgetGeneral.ui index 8885ef7c..f84c6a55 100644 --- a/src/gui/ApplicationSettingsWidgetGeneral.ui +++ b/src/gui/ApplicationSettingsWidgetGeneral.ui @@ -470,7 +470,7 @@ - + 0 0 @@ -490,6 +490,13 @@ + + + + (restart program to activate) + + + From 71e375aff0bea60233fd1907011a1c4a576c18e9 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sun, 7 Apr 2019 12:00:02 -0400 Subject: [PATCH 13/19] Allow copying passwords directly from searching * Reverts removal of previously implemented feature * Fix #2630 * Make gui search tests more robust --- src/gui/SearchWidget.cpp | 8 +++++++- tests/gui/TestGui.cpp | 24 ++++++++++++++++++------ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/gui/SearchWidget.cpp b/src/gui/SearchWidget.cpp index 6e9b6692..5667852b 100644 --- a/src/gui/SearchWidget.cpp +++ b/src/gui/SearchWidget.cpp @@ -1,5 +1,4 @@ /* - * Copyright (C) 2016 Jonathan White * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify @@ -97,6 +96,13 @@ bool SearchWidget::eventFilter(QObject* obj, QEvent* event) if (keyEvent->key() == Qt::Key_Escape) { emit escapePressed(); return true; + } else if (keyEvent->matches(QKeySequence::Copy)) { + // If Control+C is pressed in the search edit when no text + // is selected, copy the password of the current entry. + if (!m_ui->searchEdit->hasSelectedText()) { + emit copyPressed(); + return true; + } } else if (keyEvent->matches(QKeySequence::MoveToNextLine)) { if (m_ui->searchEdit->cursorPosition() == m_ui->searchEdit->text().length()) { // If down is pressed at EOL, move the focus to the entry view diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index 12756379..7d907dc9 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -849,19 +849,31 @@ void TestGui::testSearch() QTRY_VERIFY(searchTextEdit->hasFocus()); QTest::keyClick(searchTextEdit, Qt::Key_Down); QTRY_VERIFY(entryView->hasFocus()); + auto* searchedEntry = entryView->currentEntry(); // Restore focus and search text selection QTest::keyClick(m_mainWindow.data(), Qt::Key_F, Qt::ControlModifier); QTRY_COMPARE(searchTextEdit->selectedText(), QString("someTHING")); + QTRY_VERIFY(searchTextEdit->hasFocus()); + + searchedEntry->setPassword("password"); + QClipboard* clipboard = QApplication::clipboard(); + + // Attempt password copy with selected test (should fail) + QTest::keyClick(searchTextEdit, Qt::Key_C, Qt::ControlModifier); + QVERIFY(clipboard->text() != searchedEntry->password()); + // Deselect text and confirm password copies + QTest::mouseClick(searchTextEdit, Qt::LeftButton); + QTRY_VERIFY(searchTextEdit->selectedText().isEmpty()); + QTRY_VERIFY(searchTextEdit->hasFocus()); + QTest::keyClick(searchTextEdit, Qt::Key_C, Qt::ControlModifier); + QCOMPARE(searchedEntry->password(), clipboard->text()); // Ensure Down focuses on entry view when search text is selected QTest::keyClick(searchTextEdit, Qt::Key_Down); QTRY_VERIFY(entryView->hasFocus()); - QCOMPARE(entryView->selectionModel()->currentIndex().row(), 0); - // Test that password copies (entry has focus) - QClipboard* clipboard = QApplication::clipboard(); + QCOMPARE(entryView->currentEntry(), searchedEntry); + // Test that password copies with entry focused QTest::keyClick(entryView, Qt::Key_C, Qt::ControlModifier); - QModelIndex searchedItem = entryView->model()->index(0, 1); - Entry* searchedEntry = entryView->entryFromIndex(searchedItem); - QTRY_COMPARE(searchedEntry->password(), clipboard->text()); + QCOMPARE(searchedEntry->password(), clipboard->text()); // Refocus back to search edit QTest::mouseClick(searchTextEdit, Qt::LeftButton); QTRY_VERIFY(searchTextEdit->hasFocus()); From 4b1258f5851c12e2fbe7991fff794a8e78237656 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sun, 7 Apr 2019 09:56:25 -0400 Subject: [PATCH 14/19] Correct issues with apply button * Don't show apply button when creating new entries or groups (Fix #2191) * Don't mark entry/group as dirty when first creating a new one (prevents unnecessary discard dialog on cancel) * Properly enable/disable apply button when changes are made to entries and groups * Don't show discard change warning when locking database unless their are actual changes made NOTE: Extra pages in the group edit widget are not watched for changes yet. Requires a major refactor. --- src/gui/DatabaseWidget.cpp | 11 ++- src/gui/EditWidget.cpp | 37 ++++++++- src/gui/EditWidget.h | 5 ++ src/gui/entry/EditEntryWidget.cpp | 121 ++++++++++++------------------ src/gui/entry/EditEntryWidget.h | 3 - src/gui/group/EditGroupWidget.cpp | 45 +++++++++++ src/gui/group/EditGroupWidget.h | 1 + tests/gui/TestGui.cpp | 21 ++++-- 8 files changed, 155 insertions(+), 89 deletions(-) diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index abf8fcf7..97fdeb0f 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -261,12 +261,11 @@ bool DatabaseWidget::isSearchActive() const bool DatabaseWidget::isEditWidgetModified() const { if (currentWidget() == m_editEntryWidget) { - return m_editEntryWidget->hasBeenModified(); - } else { - // other edit widget don't have a hasBeenModified() method yet - // assume that they already have been modified - return true; + return m_editEntryWidget->isModified(); + } else if (currentWidget() == m_editGroupWidget) { + return m_editGroupWidget->isModified(); } + return false; } QList DatabaseWidget::mainSplitterSizes() const @@ -1249,7 +1248,7 @@ bool DatabaseWidget::lock() clipboard()->clearCopiedText(); - if (currentMode() == DatabaseWidget::Mode::EditMode) { + if (isEditWidgetModified()) { auto result = MessageBox::question(this, tr("Lock Database?"), tr("You are editing an entry. Discard changes and lock anyway?"), diff --git a/src/gui/EditWidget.cpp b/src/gui/EditWidget.cpp index be7ea01d..f7030c9d 100644 --- a/src/gui/EditWidget.cpp +++ b/src/gui/EditWidget.cpp @@ -30,6 +30,7 @@ EditWidget::EditWidget(QWidget* parent) { m_ui->setupUi(this); setReadOnly(false); + setModified(false); m_ui->messageWidget->setHidden(true); @@ -43,6 +44,7 @@ EditWidget::EditWidget(QWidget* parent) connect(m_ui->buttonBox, SIGNAL(accepted()), SIGNAL(accepted())); connect(m_ui->buttonBox, SIGNAL(rejected()), SIGNAL(rejected())); + connect(m_ui->buttonBox, SIGNAL(clicked(QAbstractButton*)), SLOT(buttonClicked(QAbstractButton*))); } EditWidget::~EditWidget() @@ -106,9 +108,6 @@ void EditWidget::setReadOnly(bool readOnly) m_ui->buttonBox->setStandardButtons(QDialogButtonBox::Close); } else { m_ui->buttonBox->setStandardButtons(QDialogButtonBox::Ok | QDialogButtonBox::Cancel | QDialogButtonBox::Apply); - // Find and connect the apply button - QPushButton* applyButton = m_ui->buttonBox->button(QDialogButtonBox::Apply); - connect(applyButton, SIGNAL(clicked()), SIGNAL(apply())); } } @@ -117,6 +116,17 @@ bool EditWidget::readOnly() const return m_readOnly; } +void EditWidget::setModified(bool state) +{ + m_modified = state; + enableApplyButton(state); +} + +bool EditWidget::isModified() const +{ + return m_modified; +} + void EditWidget::enableApplyButton(bool enabled) { QPushButton* applyButton = m_ui->buttonBox->button(QDialogButtonBox::Apply); @@ -125,6 +135,27 @@ void EditWidget::enableApplyButton(bool enabled) } } +void EditWidget::showApplyButton(bool state) +{ + if (!m_readOnly) { + auto buttons = m_ui->buttonBox->standardButtons(); + if (state) { + buttons |= QDialogButtonBox::Apply; + } else { + buttons &= ~QDialogButtonBox::Apply; + } + m_ui->buttonBox->setStandardButtons(buttons); + } +} + +void EditWidget::buttonClicked(QAbstractButton* button) +{ + auto stdButton = m_ui->buttonBox->standardButton(button); + if (stdButton == QDialogButtonBox::Apply) { + emit apply(); + } +} + void EditWidget::showMessage(const QString& text, MessageWidget::MessageType type) { // Show error messages for a longer time to make sure the user can read them diff --git a/src/gui/EditWidget.h b/src/gui/EditWidget.h index f0d157c4..361961f7 100644 --- a/src/gui/EditWidget.h +++ b/src/gui/EditWidget.h @@ -49,6 +49,8 @@ public: void setReadOnly(bool readOnly); bool readOnly() const; void enableApplyButton(bool enabled); + void showApplyButton(bool state); + virtual bool isModified() const; signals: void apply(); @@ -58,10 +60,13 @@ signals: protected slots: void showMessage(const QString& text, MessageWidget::MessageType type); void hideMessage(); + void setModified(bool state = true); + void buttonClicked(QAbstractButton* button); private: const QScopedPointer m_ui; bool m_readOnly; + bool m_modified; Q_DISABLE_COPY(EditWidget) }; diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index 74a1fcd3..508f3c68 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -276,55 +276,54 @@ void EditEntryWidget::setupHistory() void EditEntryWidget::setupEntryUpdate() { // Entry tab - connect(m_mainUi->titleEdit, SIGNAL(textChanged(QString)), this, SLOT(setUnsavedChanges())); - connect(m_mainUi->usernameEdit, SIGNAL(textChanged(QString)), this, SLOT(setUnsavedChanges())); - connect(m_mainUi->passwordEdit, SIGNAL(textChanged(QString)), this, SLOT(setUnsavedChanges())); - connect(m_mainUi->passwordRepeatEdit, SIGNAL(textChanged(QString)), this, SLOT(setUnsavedChanges())); - connect(m_mainUi->urlEdit, SIGNAL(textChanged(QString)), this, SLOT(setUnsavedChanges())); + connect(m_mainUi->titleEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); + connect(m_mainUi->usernameEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); + connect(m_mainUi->passwordEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); + connect(m_mainUi->passwordRepeatEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); + connect(m_mainUi->urlEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); #ifdef WITH_XC_NETWORKING connect(m_mainUi->urlEdit, SIGNAL(textChanged(QString)), this, SLOT(updateFaviconButtonEnable(QString))); #endif - connect(m_mainUi->expireCheck, SIGNAL(stateChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_mainUi->notesEnabled, SIGNAL(stateChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_mainUi->expireDatePicker, SIGNAL(dateTimeChanged(QDateTime)), this, SLOT(setUnsavedChanges())); - connect(m_mainUi->notesEdit, SIGNAL(textChanged()), this, SLOT(setUnsavedChanges())); + connect(m_mainUi->expireCheck, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_mainUi->notesEnabled, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_mainUi->expireDatePicker, SIGNAL(dateTimeChanged(QDateTime)), this, SLOT(setModified())); + connect(m_mainUi->notesEdit, SIGNAL(textChanged()), this, SLOT(setModified())); // Advanced tab - connect(m_advancedUi->attributesEdit, SIGNAL(textChanged()), this, SLOT(setUnsavedChanges())); - connect(m_advancedUi->protectAttributeButton, SIGNAL(stateChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_advancedUi->fgColorCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_advancedUi->bgColorCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_advancedUi->attachmentsWidget, SIGNAL(widgetUpdated()), this, SLOT(setUnsavedChanges())); + connect(m_advancedUi->attributesEdit, SIGNAL(textChanged()), this, SLOT(setModified())); + connect(m_advancedUi->protectAttributeButton, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_advancedUi->fgColorCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_advancedUi->bgColorCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_advancedUi->attachmentsWidget, SIGNAL(widgetUpdated()), this, SLOT(setModified())); // Icon tab - connect(m_iconsWidget, SIGNAL(widgetUpdated()), this, SLOT(setUnsavedChanges())); + connect(m_iconsWidget, SIGNAL(widgetUpdated()), this, SLOT(setModified())); // Auto-Type tab - connect(m_autoTypeUi->enableButton, SIGNAL(stateChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_autoTypeUi->customWindowSequenceButton, SIGNAL(stateChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_autoTypeUi->inheritSequenceButton, SIGNAL(toggled(bool)), this, SLOT(setUnsavedChanges())); - connect(m_autoTypeUi->customSequenceButton, SIGNAL(toggled(bool)), this, SLOT(setUnsavedChanges())); - connect(m_autoTypeUi->windowSequenceEdit, SIGNAL(textChanged(QString)), this, SLOT(setUnsavedChanges())); - connect(m_autoTypeUi->sequenceEdit, SIGNAL(textChanged(QString)), this, SLOT(setUnsavedChanges())); - connect(m_autoTypeUi->windowTitleCombo, SIGNAL(currentIndexChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_autoTypeUi->windowTitleCombo, SIGNAL(editTextChanged(QString)), this, SLOT(setUnsavedChanges())); + connect(m_autoTypeUi->enableButton, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_autoTypeUi->customWindowSequenceButton, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_autoTypeUi->inheritSequenceButton, SIGNAL(toggled(bool)), this, SLOT(setModified())); + connect(m_autoTypeUi->customSequenceButton, SIGNAL(toggled(bool)), this, SLOT(setModified())); + connect(m_autoTypeUi->windowSequenceEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); + connect(m_autoTypeUi->sequenceEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); + connect(m_autoTypeUi->windowTitleCombo, SIGNAL(currentIndexChanged(int)), this, SLOT(setModified())); + connect(m_autoTypeUi->windowTitleCombo, SIGNAL(editTextChanged(QString)), this, SLOT(setModified())); // Properties and History tabs don't need extra connections #ifdef WITH_XC_SSHAGENT // SSH Agent tab if (config()->get("SSHAgent", false).toBool()) { - connect(m_sshAgentUi->attachmentRadioButton, SIGNAL(toggled(bool)), this, SLOT(setUnsavedChanges())); - connect(m_sshAgentUi->externalFileRadioButton, SIGNAL(toggled(bool)), this, SLOT(setUnsavedChanges())); - connect(m_sshAgentUi->attachmentComboBox, SIGNAL(currentIndexChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_sshAgentUi->attachmentComboBox, SIGNAL(editTextChanged(QString)), this, SLOT(setUnsavedChanges())); - connect(m_sshAgentUi->externalFileEdit, SIGNAL(textChanged(QString)), this, SLOT(setUnsavedChanges())); - connect(m_sshAgentUi->addKeyToAgentCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_sshAgentUi->removeKeyFromAgentCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setUnsavedChanges())); - connect( - m_sshAgentUi->requireUserConfirmationCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_sshAgentUi->lifetimeCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setUnsavedChanges())); - connect(m_sshAgentUi->lifetimeSpinBox, SIGNAL(valueChanged(int)), this, SLOT(setUnsavedChanges())); + connect(m_sshAgentUi->attachmentRadioButton, SIGNAL(toggled(bool)), this, SLOT(setModified())); + connect(m_sshAgentUi->externalFileRadioButton, SIGNAL(toggled(bool)), this, SLOT(setModified())); + connect(m_sshAgentUi->attachmentComboBox, SIGNAL(currentIndexChanged(int)), this, SLOT(setModified())); + connect(m_sshAgentUi->attachmentComboBox, SIGNAL(editTextChanged(QString)), this, SLOT(setModified())); + connect(m_sshAgentUi->externalFileEdit, SIGNAL(textChanged(QString)), this, SLOT(setModified())); + connect(m_sshAgentUi->addKeyToAgentCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_sshAgentUi->removeKeyFromAgentCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_sshAgentUi->requireUserConfirmationCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_sshAgentUi->lifetimeCheckBox, SIGNAL(stateChanged(int)), this, SLOT(setModified())); + connect(m_sshAgentUi->lifetimeSpinBox, SIGNAL(valueChanged(int)), this, SLOT(setModified())); } #endif } @@ -703,8 +702,10 @@ void EditEntryWidget::loadEntry(Entry* entry, setCurrentPage(0); setPageHidden(m_historyWidget, m_history || m_entry->historyItems().count() < 1); - // Force the user to Save/Apply/Discard new entries - setUnsavedChanges(m_create); + // Force the user to Save/Discard new entries + showApplyButton(!m_create); + + setModified(false); } void EditEntryWidget::setForms(Entry* entry, bool restore) @@ -881,7 +882,6 @@ bool EditEntryWidget::commitEntry() } updateEntryData(m_entry); - setUnsavedChanges(false); if (!m_create) { m_entry->endUpdate(); @@ -896,6 +896,7 @@ bool EditEntryWidget::commitEntry() m_historyModel->setEntries(m_entry->historyItems()); showMessage(tr("Entry updated successfully."), MessageWidget::Positive); + setModified(false); return true; } @@ -968,7 +969,7 @@ void EditEntryWidget::cancel() m_entry->setIcon(Entry::DefaultIconNumber); } - if (!m_saved) { + if (isModified()) { auto result = MessageBox::question(this, QString(), tr("Entry has unsaved changes"), @@ -980,13 +981,13 @@ void EditEntryWidget::cancel() } if (result == MessageBox::Save) { commitEntry(); - m_saved = true; + setModified(false); } } clear(); - emit editFinished(m_saved); + emit editFinished(!isModified()); } void EditEntryWidget::clear() @@ -1008,22 +1009,6 @@ void EditEntryWidget::clear() hideMessage(); } -bool EditEntryWidget::hasBeenModified() const -{ - // entry has been modified if a history item is to be deleted - if (!m_historyModel->deletedEntries().isEmpty()) { - return true; - } - - // check if updating the entry would modify it - auto* entry = new Entry(); - entry->copyDataFrom(m_entry.data()); - - entry->beginUpdate(); - updateEntryData(entry); - return entry->endUpdate(); -} - void EditEntryWidget::togglePasswordGeneratorButton(bool checked) { if (checked) { @@ -1070,7 +1055,7 @@ void EditEntryWidget::insertAttribute() m_advancedUi->attributesView->setCurrentIndex(index); m_advancedUi->attributesView->edit(index); - setUnsavedChanges(true); + setModified(true); } void EditEntryWidget::editCurrentAttribute() @@ -1081,7 +1066,7 @@ void EditEntryWidget::editCurrentAttribute() if (index.isValid()) { m_advancedUi->attributesView->edit(index); - setUnsavedChanges(true); + setModified(true); } } @@ -1101,7 +1086,7 @@ void EditEntryWidget::removeCurrentAttribute() if (result == MessageBox::Remove) { m_entryAttributes->remove(m_attributesModel->keyByIndex(index)); - setUnsavedChanges(true); + setModified(true); } } } @@ -1223,7 +1208,7 @@ void EditEntryWidget::insertAutoTypeAssoc() m_autoTypeUi->assocView->setCurrentIndex(newIndex); loadCurrentAssoc(newIndex); m_autoTypeUi->windowTitleCombo->setFocus(); - setUnsavedChanges(true); + setModified(true); } void EditEntryWidget::removeAutoTypeAssoc() @@ -1232,7 +1217,7 @@ void EditEntryWidget::removeAutoTypeAssoc() if (currentIndex.isValid()) { m_autoTypeAssoc->remove(currentIndex.row()); - setUnsavedChanges(true); + setModified(true); } } @@ -1295,7 +1280,7 @@ void EditEntryWidget::restoreHistoryEntry() QModelIndex index = m_sortModel->mapToSource(m_historyUi->historyView->currentIndex()); if (index.isValid()) { setForms(m_historyModel->entryFromIndex(index), true); - setUnsavedChanges(true); + setModified(true); } } @@ -1309,7 +1294,7 @@ void EditEntryWidget::deleteHistoryEntry() } else { m_historyUi->deleteAllButton->setEnabled(false); } - setUnsavedChanges(true); + setModified(true); } } @@ -1317,7 +1302,7 @@ void EditEntryWidget::deleteAllHistoryEntries() { m_historyModel->deleteAll(); m_historyUi->deleteAllButton->setEnabled(m_historyModel->rowCount() > 0); - setUnsavedChanges(true); + setModified(true); } QMenu* EditEntryWidget::createPresetsMenu() @@ -1370,12 +1355,6 @@ void EditEntryWidget::pickColor() QColor newColor = QColorDialog::getColor(oldColor); if (newColor.isValid()) { setupColorButton(isForeground, newColor); - setUnsavedChanges(true); + setModified(true); } } - -void EditEntryWidget::setUnsavedChanges(bool hasUnsaved) -{ - m_saved = !hasUnsaved; - enableApplyButton(hasUnsaved); -} diff --git a/src/gui/entry/EditEntryWidget.h b/src/gui/entry/EditEntryWidget.h index 4c687059..aea3c894 100644 --- a/src/gui/entry/EditEntryWidget.h +++ b/src/gui/entry/EditEntryWidget.h @@ -68,7 +68,6 @@ public: QString entryTitle() const; void clear(); - bool hasBeenModified() const; signals: void editFinished(bool accepted); @@ -106,7 +105,6 @@ private slots: void useExpiryPreset(QAction* action); void toggleHideNotes(bool visible); void pickColor(); - void setUnsavedChanges(bool hasUnsaved = true); #ifdef WITH_XC_SSHAGENT void updateSSHAgent(); void updateSSHAgentAttachment(); @@ -148,7 +146,6 @@ private: bool m_create; bool m_history; - bool m_saved; #ifdef WITH_XC_SSHAGENT bool m_sshAgentEnabled; KeeAgentSettings m_sshAgentSettings; diff --git a/src/gui/group/EditGroupWidget.cpp b/src/gui/group/EditGroupWidget.cpp index 6c869cf2..fe83a943 100644 --- a/src/gui/group/EditGroupWidget.cpp +++ b/src/gui/group/EditGroupWidget.cpp @@ -22,6 +22,7 @@ #include "core/Metadata.h" #include "gui/EditWidgetIcons.h" #include "gui/EditWidgetProperties.h" +#include "gui/MessageBox.h" #if defined(WITH_XC_KEESHARE) #include "keeshare/group/EditGroupPageKeeShare.h" @@ -46,6 +47,11 @@ public: editPage->assign(widget); } + QWidget* getWidget() + { + return widget; + } + private: QSharedPointer editPage; QWidget* widget; @@ -85,18 +91,38 @@ EditGroupWidget::EditGroupWidget(QWidget* parent) // clang-format on connect(m_editGroupWidgetIcons, SIGNAL(messageEditEntryDismiss()), SLOT(hideMessage())); + + setupModifiedTracking(); } EditGroupWidget::~EditGroupWidget() { } +void EditGroupWidget::setupModifiedTracking() +{ + // Group tab + connect(m_mainUi->editName, SIGNAL(textChanged(QString)), SLOT(setModified())); + connect(m_mainUi->editNotes, SIGNAL(textChanged()), SLOT(setModified())); + connect(m_mainUi->expireCheck, SIGNAL(stateChanged(int)), SLOT(setModified())); + connect(m_mainUi->expireDatePicker, SIGNAL(dateTimeChanged(QDateTime)), SLOT(setModified())); + connect(m_mainUi->searchComboBox, SIGNAL(currentIndexChanged(int)), SLOT(setModified())); + connect(m_mainUi->autotypeComboBox, SIGNAL(currentIndexChanged(int)), SLOT(setModified())); + connect(m_mainUi->autoTypeSequenceInherit, SIGNAL(toggled(bool)), SLOT(setModified())); + connect(m_mainUi->autoTypeSequenceCustomRadio, SIGNAL(toggled(bool)), SLOT(setModified())); + connect(m_mainUi->autoTypeSequenceCustomEdit, SIGNAL(textChanged(QString)), SLOT(setModified())); + + // Icon tab + connect(m_editGroupWidgetIcons, SIGNAL(widgetUpdated()), SLOT(setModified())); +} + void EditGroupWidget::loadGroup(Group* group, bool create, const QSharedPointer& database) { m_group = group; m_db = database; m_temporaryGroup.reset(group->clone(Entry::CloneNoFlags, Group::CloneNoFlags)); + connect(m_temporaryGroup->customData(), SIGNAL(customDataModified()), SLOT(setModified())); if (create) { setHeadline(tr("Add group")); @@ -139,6 +165,11 @@ void EditGroupWidget::loadGroup(Group* group, bool create, const QSharedPointer< setCurrentPage(0); m_mainUi->editName->setFocus(); + + // Force the user to Save/Discard new groups + showApplyButton(!create); + + setModified(false); } void EditGroupWidget::save() @@ -180,6 +211,8 @@ void EditGroupWidget::apply() // Icons add/remove are applied globally outside the transaction! m_group->copyDataFrom(m_temporaryGroup.data()); + + setModified(false); } void EditGroupWidget::cancel() @@ -188,6 +221,18 @@ void EditGroupWidget::cancel() m_group->setIcon(Entry::DefaultIconNumber); } + if (isModified()) { + auto result = MessageBox::question(this, + QString(), + tr("Entry has unsaved changes"), + MessageBox::Cancel | MessageBox::Save | MessageBox::Discard, + MessageBox::Cancel); + if (result == MessageBox::Save) { + apply(); + setModified(false); + } + } + clear(); emit editFinished(false); } diff --git a/src/gui/group/EditGroupWidget.h b/src/gui/group/EditGroupWidget.h index fd744503..cc8738d8 100644 --- a/src/gui/group/EditGroupWidget.h +++ b/src/gui/group/EditGroupWidget.h @@ -74,6 +74,7 @@ private: void addTriStateItems(QComboBox* comboBox, bool inheritValue); int indexFromTriState(Group::TriState triState); Group::TriState triStateFromIndex(int index); + void setupModifiedTracking(); const QScopedPointer m_mainUi; diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index 7d907dc9..63e36fba 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -429,13 +429,20 @@ void TestGui::testEditEntry() auto* titleEdit = editEntryWidget->findChild("titleEdit"); QTest::keyClicks(titleEdit, "_test"); - // Apply the edit auto* editEntryWidgetButtonBox = editEntryWidget->findChild("buttonBox"); QVERIFY(editEntryWidgetButtonBox); - QTest::mouseClick(editEntryWidgetButtonBox->button(QDialogButtonBox::Apply), Qt::LeftButton); + auto* okButton = editEntryWidgetButtonBox->button(QDialogButtonBox::Ok); + QVERIFY(okButton); + auto* applyButton = editEntryWidgetButtonBox->button(QDialogButtonBox::Apply); + QVERIFY(applyButton); + + // Apply the edit + QTRY_VERIFY(applyButton->isEnabled()); + QTest::mouseClick(applyButton, Qt::LeftButton); QCOMPARE(m_dbWidget->currentMode(), DatabaseWidget::Mode::EditMode); QCOMPARE(entry->title(), QString("Sample Entry_test")); QCOMPARE(entry->historyItems().size(), ++editCount); + QVERIFY(!applyButton->isEnabled()); // Test entry colors (simulate choosing a color) editEntryWidget->setCurrentPage(1); @@ -451,7 +458,7 @@ void TestGui::testEditEntry() colorCheckBox = editEntryWidget->findChild("bgColorCheckBox"); colorButton->setProperty("color", bgColor); colorCheckBox->setChecked(true); - QTest::mouseClick(editEntryWidgetButtonBox->button(QDialogButtonBox::Apply), Qt::LeftButton); + QTest::mouseClick(applyButton, Qt::LeftButton); QCOMPARE(entry->historyItems().size(), ++editCount); // Test protected attributes @@ -471,7 +478,7 @@ void TestGui::testEditEntry() auto* passwordEdit = editEntryWidget->findChild("passwordEdit"); QString originalPassword = passwordEdit->text(); passwordEdit->setText("newpass"); - QTest::mouseClick(editEntryWidgetButtonBox->button(QDialogButtonBox::Ok), Qt::LeftButton); + QTest::mouseClick(okButton, Qt::LeftButton); auto* messageWiget = editEntryWidget->findChild("messageWidget"); QTRY_VERIFY(messageWiget->isVisible()); QCOMPARE(m_dbWidget->currentMode(), DatabaseWidget::Mode::EditMode); @@ -479,7 +486,7 @@ void TestGui::testEditEntry() passwordEdit->setText(originalPassword); // Save the edit (press OK) - QTest::mouseClick(editEntryWidgetButtonBox->button(QDialogButtonBox::Ok), Qt::LeftButton); + QTest::mouseClick(okButton, Qt::LeftButton); QApplication::processEvents(); // Confirm edit was made @@ -496,13 +503,15 @@ void TestGui::testEditEntry() // Test copy & paste newline sanitization QTest::mouseClick(entryEditWidget, Qt::LeftButton); + okButton = editEntryWidgetButtonBox->button(QDialogButtonBox::Ok); + QVERIFY(okButton); QCOMPARE(m_dbWidget->currentMode(), DatabaseWidget::Mode::EditMode); titleEdit->setText("multiline\ntitle"); editEntryWidget->findChild("usernameEdit")->setText("multiline\nusername"); editEntryWidget->findChild("passwordEdit")->setText("multiline\npassword"); editEntryWidget->findChild("passwordRepeatEdit")->setText("multiline\npassword"); editEntryWidget->findChild("urlEdit")->setText("multiline\nurl"); - QTest::mouseClick(editEntryWidgetButtonBox->button(QDialogButtonBox::Ok), Qt::LeftButton); + QTest::mouseClick(okButton, Qt::LeftButton); QCOMPARE(entry->title(), QString("multiline title")); QCOMPARE(entry->username(), QString("multiline username")); From 1493943e2e34e4c9c270521032023c20244bac51 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Fri, 5 Apr 2019 21:04:56 -0400 Subject: [PATCH 15/19] Add integration with Brave browser Fixes #2414 --- README.md | 2 +- src/browser/BrowserOptionDialog.cpp | 8 ++++++-- src/browser/BrowserOptionDialog.ui | 10 ++++++++++ src/browser/BrowserSettings.cpp | 11 +++++++++++ src/browser/BrowserSettings.h | 2 ++ src/browser/HostInstaller.cpp | 12 ++++++++++-- src/browser/HostInstaller.h | 4 +++- utils/keepassxc-snap-helper.sh | 18 ++++++++++++------ 8 files changed, 55 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 608dfd36..3048e2ea 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ so please check out your distribution's package list to see if KeePassXC is avai - Using website favicons as entry icons - Merging of databases - Automatic reload when the database changed on disk -- Browser integration with KeePassXC-Browser using [native messaging](https://developer.chrome.com/extensions/nativeMessaging) for [Mozilla Firefox](https://addons.mozilla.org/en-US/firefox/addon/keepassxc-browser/) and [Google Chrome or Chromium](https://chrome.google.com/webstore/detail/keepassxc-browser/oboonakemofpalcgghocfoadofidjkkk) +- Browser integration with KeePassXC-Browser using [native messaging](https://developer.chrome.com/extensions/nativeMessaging) for [Mozilla Firefox](https://addons.mozilla.org/en-US/firefox/addon/keepassxc-browser/) and [Google Chrome, Chromium, Vivaldi, or Brave](https://chrome.google.com/webstore/detail/keepassxc-browser/oboonakemofpalcgghocfoadofidjkkk) - Synchronize passwords using KeeShare. See [Using Sharing](./docs/QUICKSTART.md#using-sharing) for more details. - Many bug fixes diff --git a/src/browser/BrowserOptionDialog.cpp b/src/browser/BrowserOptionDialog.cpp index dd91f159..9eecc63f 100644 --- a/src/browser/BrowserOptionDialog.cpp +++ b/src/browser/BrowserOptionDialog.cpp @@ -47,7 +47,7 @@ BrowserOptionDialog::BrowserOptionDialog(QWidget* parent) tr("KeePassXC-Browser is needed for the browser integration to work.
Download it for %1 and %2. %3") .arg("Firefox", "" - "Google Chrome / Chromium / Vivaldi", + "Google Chrome / Chromium / Vivaldi / Brave", snapInstructions)); // clang-format on @@ -75,9 +75,11 @@ BrowserOptionDialog::BrowserOptionDialog(QWidget* parent) connect(m_ui->customProxyLocationBrowseButton, SIGNAL(clicked()), this, SLOT(showProxyLocationFileDialog())); #ifdef Q_OS_WIN + // Brave uses Chrome's registry settings + m_ui->braveSupport->setHidden(true); // Vivaldi uses Chrome's registry settings m_ui->vivaldiSupport->setHidden(true); - m_ui->chromeSupport->setText("Chrome and Vivaldi"); + m_ui->chromeSupport->setText("Chrome, Vivaldi, and Brave"); // Tor Browser uses Firefox's registry settings m_ui->torBrowserSupport->setHidden(true); m_ui->firefoxSupport->setText("Firefox and Tor Browser"); @@ -122,6 +124,7 @@ void BrowserOptionDialog::loadSettings() m_ui->chromiumSupport->setChecked(settings->chromiumSupport()); m_ui->firefoxSupport->setChecked(settings->firefoxSupport()); #ifndef Q_OS_WIN + m_ui->braveSupport->setChecked(settings->braveSupport()); m_ui->vivaldiSupport->setChecked(settings->vivaldiSupport()); m_ui->torBrowserSupport->setChecked(settings->torBrowserSupport()); #endif @@ -183,6 +186,7 @@ void BrowserOptionDialog::saveSettings() settings->setChromiumSupport(m_ui->chromiumSupport->isChecked()); settings->setFirefoxSupport(m_ui->firefoxSupport->isChecked()); #ifndef Q_OS_WIN + settings->setBraveSupport(m_ui->braveSupport->isChecked()); settings->setVivaldiSupport(m_ui->vivaldiSupport->isChecked()); settings->setTorBrowserSupport(m_ui->torBrowserSupport->isChecked()); #endif diff --git a/src/browser/BrowserOptionDialog.ui b/src/browser/BrowserOptionDialog.ui index 2b32bb9e..50fd9d20 100755 --- a/src/browser/BrowserOptionDialog.ui +++ b/src/browser/BrowserOptionDialog.ui @@ -150,6 +150,16 @@ + + + + &Brave + + + false + + + diff --git a/src/browser/BrowserSettings.cpp b/src/browser/BrowserSettings.cpp index 9aab68f7..dd74dc1c 100644 --- a/src/browser/BrowserSettings.cpp +++ b/src/browser/BrowserSettings.cpp @@ -238,6 +238,17 @@ void BrowserSettings::setVivaldiSupport(bool enabled) HostInstaller::SupportedBrowsers::VIVALDI, enabled, supportBrowserProxy(), customProxyLocation()); } +bool BrowserSettings::braveSupport() +{ + return m_hostInstaller.checkIfInstalled(HostInstaller::SupportedBrowsers::BRAVE); +} + +void BrowserSettings::setBraveSupport(bool enabled) +{ + m_hostInstaller.installBrowser( + HostInstaller::SupportedBrowsers::BRAVE, enabled, supportBrowserProxy(), customProxyLocation()); +} + bool BrowserSettings::torBrowserSupport() { return m_hostInstaller.checkIfInstalled(HostInstaller::SupportedBrowsers::TOR_BROWSER); diff --git a/src/browser/BrowserSettings.h b/src/browser/BrowserSettings.h index b00c75b7..ba74ff53 100644 --- a/src/browser/BrowserSettings.h +++ b/src/browser/BrowserSettings.h @@ -72,6 +72,8 @@ public: void setFirefoxSupport(bool enabled); bool vivaldiSupport(); void setVivaldiSupport(bool enabled); + bool braveSupport(); + void setBraveSupport(bool enabled); bool torBrowserSupport(); void setTorBrowserSupport(bool enabled); diff --git a/src/browser/HostInstaller.cpp b/src/browser/HostInstaller.cpp index 08782fa1..20c55456 100644 --- a/src/browser/HostInstaller.cpp +++ b/src/browser/HostInstaller.cpp @@ -39,12 +39,14 @@ HostInstaller::HostInstaller() , TARGET_DIR_FIREFOX("/Library/Application Support/Mozilla/NativeMessagingHosts") , TARGET_DIR_VIVALDI("/Library/Application Support/Vivaldi/NativeMessagingHosts") , TARGET_DIR_TOR_BROWSER("/Library/Application Support/TorBrowser-Data/Browser/Mozilla/NativeMessagingHosts") + , TARGET_DIR_BRAVE("/Library/Application Support/BraveSoftware/Brave-Browser/NativeMessagingHosts") #elif defined(Q_OS_LINUX) , TARGET_DIR_CHROME("/.config/google-chrome/NativeMessagingHosts") , TARGET_DIR_CHROMIUM("/.config/chromium/NativeMessagingHosts") , TARGET_DIR_FIREFOX("/.mozilla/native-messaging-hosts") , TARGET_DIR_VIVALDI("/.config/vivaldi/NativeMessagingHosts") , TARGET_DIR_TOR_BROWSER("/.tor-browser/app/Browser/TorBrowser/Data/Browser/.mozilla/native-messaging-hosts") + , TARGET_DIR_BRAVE("/.config/BraveSoftware/Brave-Browser/NativeMessagingHosts") #elif defined(Q_OS_WIN) // clang-format off , TARGET_DIR_CHROME("HKEY_CURRENT_USER\\Software\\Google\\Chrome\\NativeMessagingHosts\\org.keepassxc.keepassxc_browser") @@ -53,6 +55,7 @@ HostInstaller::HostInstaller() , TARGET_DIR_FIREFOX("HKEY_CURRENT_USER\\Software\\Mozilla\\NativeMessagingHosts\\org.keepassxc.keepassxc_browser") , TARGET_DIR_VIVALDI(TARGET_DIR_CHROME) , TARGET_DIR_TOR_BROWSER(TARGET_DIR_FIREFOX) + , TARGET_DIR_BRAVE(TARGET_DIR_CHROME) #endif { } @@ -140,7 +143,8 @@ void HostInstaller::installBrowser(SupportedBrowsers browser, */ void HostInstaller::updateBinaryPaths(const bool& proxy, const QString& location) { - for (int i = 0; i < 4; ++i) { + // Where 6 is the number of entries in the SupportedBrowsers enum declared in HostInstaller.h + for (int i = 0; i < 6; ++i) { if (checkIfInstalled(static_cast(i))) { installBrowser(static_cast(i), true, proxy, location); } @@ -166,6 +170,8 @@ QString HostInstaller::getTargetPath(SupportedBrowsers browser) const return TARGET_DIR_VIVALDI; case SupportedBrowsers::TOR_BROWSER: return TARGET_DIR_TOR_BROWSER; + case SupportedBrowsers::BRAVE: + return TARGET_DIR_BRAVE; default: return QString(); } @@ -188,9 +194,11 @@ QString HostInstaller::getBrowserName(SupportedBrowsers browser) const case SupportedBrowsers::FIREFOX: return "firefox"; case SupportedBrowsers::VIVALDI: - return "vivaldi"; + return "vivaldi"; case SupportedBrowsers::TOR_BROWSER: return "tor-browser"; + case SupportedBrowsers::BRAVE: + return "brave"; default: return QString(); } diff --git a/src/browser/HostInstaller.h b/src/browser/HostInstaller.h index ea0c4bd2..154fe21a 100644 --- a/src/browser/HostInstaller.h +++ b/src/browser/HostInstaller.h @@ -34,7 +34,8 @@ public: CHROMIUM = 1, FIREFOX = 2, VIVALDI = 3, - TOR_BROWSER = 4 + TOR_BROWSER = 4, + BRAVE = 5 }; public: @@ -66,6 +67,7 @@ private: const QString TARGET_DIR_FIREFOX; const QString TARGET_DIR_VIVALDI; const QString TARGET_DIR_TOR_BROWSER; + const QString TARGET_DIR_BRAVE; }; #endif // HOSTINSTALLER_H diff --git a/utils/keepassxc-snap-helper.sh b/utils/keepassxc-snap-helper.sh index 4b2ce94d..206accaf 100755 --- a/utils/keepassxc-snap-helper.sh +++ b/utils/keepassxc-snap-helper.sh @@ -92,6 +92,11 @@ setupVivaldi() { INSTALL_DIR="${BASE_DIR}/.config/vivaldi/NativeMessagingHosts" } +setupBrave() { + buildJson + INSTALL_DIR="${BASE_DIR}/.config/BraveSoftware/Brave-Browser/NativeMessagingHosts" +} + setupTorBrowser() { buildJson "firefox" INSTALL_DIR="${BASE_DIR}/.tor-browser/app/Browser/TorBrowser/Data/Browser/.mozilla/native-messaging-hosts" @@ -109,9 +114,10 @@ BROWSER=$(whiptail \ "2" "Chrome" \ "3" "Chromium" \ "4" "Vivaldi" \ - "5" "Tor Browser" \ + "5" "Brave" \ + "6" "Tor Browser" \ 3>&1 1>&2 2>&3) - + clear exitstatus=$? @@ -122,16 +128,17 @@ if [ $exitstatus = 0 ]; then 2) setupChrome ;; 3) setupChromium ;; 4) setupVivaldi ;; - 5) setupTorBrowser ;; + 5) setupBrave ;; + 6) setupTorBrowser ;; esac # Install the JSON file cd ~ mkdir -p "$INSTALL_DIR" echo "$JSON_OUT" > ${INSTALL_DIR}/${INSTALL_FILE} - + $DEBUG && echo "Installed to: ${INSTALL_DIR}/${INSTALL_FILE}" - + whiptail \ --title "Installation Complete" \ --msgbox "You will need to restart your browser in order to connect to KeePassXC" \ @@ -139,4 +146,3 @@ if [ $exitstatus = 0 ]; then else whiptail --title "Installation Canceled" --msgbox "No changes were made to your system" 8 50 fi - From 29c79c935ab80095b510020fe77e68d01377473b Mon Sep 17 00:00:00 2001 From: ckieschnick Date: Mon, 8 Apr 2019 04:19:51 +0200 Subject: [PATCH 16/19] More detailed KeeShare sharing messages (#2946) * ShareObserver watches all shares ShareObserver watches all shares to and considers settings only on checking for changes. This fixes an assertion when an export group signal is received, but export was disabled. * Extend share message in group view Extended the message for shared groups to indicate deactivate import/export and errors when the share was not correctly configured. --- src/keeshare/KeeShare.cpp | 26 ++++++++++++++++++++------ src/keeshare/ShareObserver.cpp | 5 +---- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/keeshare/KeeShare.cpp b/src/keeshare/KeeShare.cpp index 01d11f02..d1cbde09 100644 --- a/src/keeshare/KeeShare.cpp +++ b/src/keeshare/KeeShare.cpp @@ -162,18 +162,32 @@ QString KeeShare::sharingLabel(const Group* group) } const auto reference = referenceOf(share); + if (!reference.isValid()) { + return tr("Invalid sharing reference"); + } + QStringList messages; switch (reference.type) { case KeeShareSettings::Inactive: - return tr("Inactive share %1").arg(reference.path); + messages << tr("Inactive share %1").arg(reference.path); + break; case KeeShareSettings::ImportFrom: - return tr("Imported from %1").arg(reference.path); + messages << tr("Imported from %1").arg(reference.path); + break; case KeeShareSettings::ExportTo: - return tr("Exported to %1").arg(reference.path); + messages << tr("Exported to %1").arg(reference.path); + break; case KeeShareSettings::SynchronizeWith: - return tr("Synchronized with %1").arg(reference.path); + messages << tr("Synchronized with %1").arg(reference.path); + break; } - - return {}; + const auto active = KeeShare::active(); + if (reference.isImporting() && !active.in) { + messages << tr("Import is disabled in settings"); + } + if (reference.isExporting() && !active.out) { + messages << tr("Export is disabled in settings"); + } + return messages.join("\n"); } QPixmap KeeShare::indicatorBadge(const Group* group, QPixmap pixmap) diff --git a/src/keeshare/ShareObserver.cpp b/src/keeshare/ShareObserver.cpp index 82d02a7d..295883ab 100644 --- a/src/keeshare/ShareObserver.cpp +++ b/src/keeshare/ShareObserver.cpp @@ -190,7 +190,6 @@ void ShareObserver::reinitialize() KeeShareSettings::Reference newReference; }; - const auto active = KeeShare::active(); QList updated; const QList groups = m_db->rootGroup()->groupsRecursive(true); for (Group* group : groups) { @@ -202,9 +201,7 @@ void ShareObserver::reinitialize() m_groupToReference.remove(couple.group); m_referenceToGroup.remove(couple.oldReference); m_shareToGroup.remove(couple.oldReference.path); - if (couple.newReference.isValid() - && ((active.in && couple.newReference.isImporting()) - || (active.out && couple.newReference.isExporting()))) { + if (couple.newReference.isValid()) { m_groupToReference[couple.group] = couple.newReference; m_referenceToGroup[couple.newReference] = couple.group; m_shareToGroup[couple.newReference.path] = couple.group; From 8bc94874a1a2e39a9d4d63600d569bb8edf363b6 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Thu, 21 Mar 2019 16:21:45 -0400 Subject: [PATCH 17/19] Enhance release-tool handling of app signing * Introduce .gitrev file to tarball generation * Correct labeling of builds based on supplied parameters to CMake * Convert supplied key file path to absolute when building under MSYS * Support OVERRIDE_VERSION to build properly version numbered snapshots * Do not build tests for any build --- CMakeLists.txt | 20 +++++++++++++------- release-tool | 33 +++++++++++++++++++-------------- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 969b3727..3e792857 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -80,6 +80,7 @@ set(KEEPASSXC_VERSION_MAJOR "2") set(KEEPASSXC_VERSION_MINOR "4") set(KEEPASSXC_VERSION_PATCH "0") set(KEEPASSXC_VERSION "${KEEPASSXC_VERSION_MAJOR}.${KEEPASSXC_VERSION_MINOR}.${KEEPASSXC_VERSION_PATCH}") +set(OVERRIDE_VERSION "" CACHE STRING "Override the KeePassXC Version for Snapshot builds") set(KEEPASSXC_BUILD_TYPE "Snapshot" CACHE STRING "Set KeePassXC build type to distinguish between stable releases and snapshots") set_property(CACHE KEEPASSXC_BUILD_TYPE PROPERTY STRINGS Snapshot Release PreRelease) @@ -91,8 +92,10 @@ execute_process(COMMAND git rev-parse --short=7 HEAD OUTPUT_VARIABLE GIT_HEAD ERROR_QUIET) string(STRIP "${GIT_HEAD}" GIT_HEAD) -if(GIT_HEAD STREQUAL "") +if(GIT_HEAD STREQUAL "" AND NOT GIT_HEAD_OVERRIDE STREQUAL "") string(SUBSTRING "${GIT_HEAD_OVERRIDE}" 0 7 GIT_HEAD) +elseif(EXISTS ${CMAKE_SOURCE_DIR}/.gitrev) + file(READ ${CMAKE_SOURCE_DIR}/.gitrev GIT_HEAD) endif() message(STATUS "Found Git HEAD Revision: ${GIT_HEAD}\n") @@ -116,13 +119,16 @@ if(OVERRIDE_VERSION) elseif(OVERRIDE_VERSION MATCHES "^[\\.0-9]+$") set(KEEPASSXC_BUILD_TYPE Release) set(KEEPASSXC_VERSION ${OVERRIDE_VERSION}) + else() + set(KEEPASSXC_BUILD_TYPE Snapshot) + set(KEEPASSXC_VERSION ${OVERRIDE_VERSION}) + endif() +else() + if(KEEPASSXC_BUILD_TYPE STREQUAL "PreRelease") + set(KEEPASSXC_VERSION "${KEEPASSXC_VERSION}-preview") + elseif(KEEPASSXC_BUILD_TYPE STREQUAL "Snapshot") + set(KEEPASSXC_VERSION "${KEEPASSXC_VERSION}-snapshot") endif() -endif() - -if(KEEPASSXC_BUILD_TYPE STREQUAL "PreRelease" AND NOT OVERRIDE_VERSION) - set(KEEPASSXC_VERSION "${KEEPASSXC_VERSION}-preview") -elseif(KEEPASSXC_BUILD_TYPE STREQUAL "Snapshot") - set(KEEPASSXC_VERSION "${KEEPASSXC_VERSION}-snapshot") endif() if(KEEPASSXC_BUILD_TYPE STREQUAL "Release") diff --git a/release-tool b/release-tool index ab4128a3..821a1e8c 100755 --- a/release-tool +++ b/release-tool @@ -813,13 +813,17 @@ build() { init OUTPUT_DIR="$(realpath "$OUTPUT_DIR")" + # Resolve appsign key to absolute path if under Windows + if [[ "${build_key}" && "$(uname -o)" == "Msys" ]]; then + build_key="$(realpath "${build_key}")" + fi if ${build_snapshot}; then TAG_NAME="HEAD" local branch=`git rev-parse --abbrev-ref HEAD` logInfo "Using current branch ${branch} to build..." RELEASE_NAME="${RELEASE_NAME}-snapshot" - CMAKE_OPTIONS="${CMAKE_OPTIONS} -DKEEPASSXC_BUILD_TYPE=Snapshot" + CMAKE_OPTIONS="${CMAKE_OPTIONS} -DKEEPASSXC_BUILD_TYPE=Snapshot -DOVERRIDE_VERSION=${RELEASE_NAME}" else checkWorkingTreeClean @@ -848,14 +852,13 @@ build() { git archive --format=tar "$TAG_NAME" --prefix="${prefix}/" --output="${OUTPUT_DIR}/${tarball_name}" - if ! ${build_snapshot}; then - # add .version file to tar - mkdir "${prefix}" - echo -n ${RELEASE_NAME} > "${prefix}/.version" - tar --append --file="${OUTPUT_DIR}/${tarball_name}" "${prefix}/.version" - rm "${prefix}/.version" - rmdir "${prefix}" 2> /dev/null - fi + # add .version and .gitrev files to tarball + mkdir "${prefix}" + echo -n ${RELEASE_NAME} > "${prefix}/.version" + echo -n `git rev-parse --short=7 HEAD` > "${prefix}/.gitrev" + tar --append --file="${OUTPUT_DIR}/${tarball_name}" "${prefix}/.version" "${prefix}/.gitrev" + rm "${prefix}/.version" "${prefix}/.gitrev" + rmdir "${prefix}" 2> /dev/null xz -6 "${OUTPUT_DIR}/${tarball_name}" fi @@ -881,6 +884,8 @@ build() { # linuxdeploy requires /usr as install prefix INSTALL_PREFIX="/usr" fi + # Do not build tests cases + CMAKE_OPTIONS="${CMAKE_OPTIONS} -DWITH_TESTS=OFF" if [ "$COMPILER" == "g++" ]; then export CC=gcc @@ -913,14 +918,14 @@ build() { elif [ "$(uname -o)" == "Msys" ]; then # Building on Windows with Msys2 logInfo "Configuring build..." - cmake -DCMAKE_BUILD_TYPE=Release -DWITH_TESTS=Off -G"MSYS Makefiles" \ + cmake -DCMAKE_BUILD_TYPE=Release -G"MSYS Makefiles" \ -DCMAKE_INSTALL_PREFIX="${INSTALL_PREFIX}" ${CMAKE_OPTIONS} "$SRC_DIR" logInfo "Compiling and packaging sources..." mingw32-make ${MAKE_OPTIONS} preinstall # Appsign the executables if desired - if ${build_appsign} && [ -f ${build_key} ]; then + if ${build_appsign} && [ -f "${build_key}" ]; then logInfo "Signing executable files" appsign "-f" $(find src | grep -P '\.exe$|\.dll$') "-k" "${build_key}" fi @@ -945,7 +950,7 @@ build() { # Building on Linux without Docker container logInfo "Configuring build..." - cmake -DCMAKE_BUILD_TYPE=Release -DWITH_TESTS=Off ${CMAKE_OPTIONS} \ + cmake -DCMAKE_BUILD_TYPE=Release ${CMAKE_OPTIONS} \ -DCMAKE_INSTALL_PREFIX="${INSTALL_PREFIX}" "$SRC_DIR" logInfo "Compiling sources..." @@ -977,7 +982,7 @@ build() { -v "$(realpath "$OUTPUT_DIR"):/keepassxc/out:rw" \ "$DOCKER_IMAGE" \ bash -c "cd /keepassxc/out/build-release && \ - cmake -DCMAKE_BUILD_TYPE=Release -DWITH_TESTS=Off ${CMAKE_OPTIONS} \ + cmake -DCMAKE_BUILD_TYPE=Release ${CMAKE_OPTIONS} \ -DCMAKE_INSTALL_PREFIX=${INSTALL_PREFIX} /keepassxc/src && \ make ${MAKE_OPTIONS} && make DESTDIR=/keepassxc/out/KeePassXC.AppDir install/strip" fi @@ -1139,7 +1144,7 @@ appsign() { fi logInfo "Signing app using codesign..." - codesign --sign "${key}" --verbose --deep --entitlements ${orig_dir}/share/macosx/keepassxc.entitlements ./app/KeePassXC.app + codesign --sign "${key}" --verbose --deep --entitlements "${SRC_DIR}/share/macosx/keepassxc.entitlements" ./app/KeePassXC.app if [ 0 -ne $? ]; then cd "${orig_dir}" From 53a57ee8c7b0f6ebb9ec89321a005833a1b5f9ba Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Tue, 9 Apr 2019 21:32:10 -0400 Subject: [PATCH 18/19] Hide window when performing entry auto-type on macOS * Instead of choosing the last active window, always hide the current window (ie, KeePassXC) * Fixes #2883 --- src/autotype/AutoType.cpp | 2 +- src/autotype/AutoTypePlatformPlugin.h | 2 +- src/autotype/mac/AutoTypeMac.cpp | 4 ++-- src/autotype/mac/AutoTypeMac.h | 2 +- src/autotype/test/AutoTypeTest.cpp | 2 +- src/autotype/test/AutoTypeTest.h | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/autotype/AutoType.cpp b/src/autotype/AutoType.cpp index 012dee62..0f772d8d 100644 --- a/src/autotype/AutoType.cpp +++ b/src/autotype/AutoType.cpp @@ -214,7 +214,7 @@ void AutoType::executeAutoTypeActions(const Entry* entry, QWidget* hideWindow, c if (hideWindow) { #if defined(Q_OS_MACOS) - m_plugin->raiseLastActiveWindow(); + m_plugin->hideOwnWindow(); #else hideWindow->showMinimized(); #endif diff --git a/src/autotype/AutoTypePlatformPlugin.h b/src/autotype/AutoTypePlatformPlugin.h index 68cf99be..059e7e13 100644 --- a/src/autotype/AutoTypePlatformPlugin.h +++ b/src/autotype/AutoTypePlatformPlugin.h @@ -43,7 +43,7 @@ public: virtual AutoTypeExecutor* createExecutor() = 0; #if defined(Q_OS_MACOS) - virtual bool raiseLastActiveWindow() = 0; + virtual bool hideOwnWindow() = 0; virtual bool raiseOwnWindow() = 0; #endif diff --git a/src/autotype/mac/AutoTypeMac.cpp b/src/autotype/mac/AutoTypeMac.cpp index 60cec114..e73e5377 100644 --- a/src/autotype/mac/AutoTypeMac.cpp +++ b/src/autotype/mac/AutoTypeMac.cpp @@ -165,9 +165,9 @@ bool AutoTypePlatformMac::raiseWindow(WId pid) // // Activate last active window // -bool AutoTypePlatformMac::raiseLastActiveWindow() +bool AutoTypePlatformMac::hideOwnWindow() { - return macUtils()->raiseLastActiveWindow(); + return macUtils()->hideOwnWindow(); } // diff --git a/src/autotype/mac/AutoTypeMac.h b/src/autotype/mac/AutoTypeMac.h index 875c2176..55963da5 100644 --- a/src/autotype/mac/AutoTypeMac.h +++ b/src/autotype/mac/AutoTypeMac.h @@ -44,7 +44,7 @@ public: bool raiseWindow(WId pid) override; AutoTypeExecutor* createExecutor() override; - bool raiseLastActiveWindow() override; + bool hideOwnWindow() override; bool raiseOwnWindow() override; void sendChar(const QChar& ch, bool isKeyDown); diff --git a/src/autotype/test/AutoTypeTest.cpp b/src/autotype/test/AutoTypeTest.cpp index f8754ef3..9a1b6501 100644 --- a/src/autotype/test/AutoTypeTest.cpp +++ b/src/autotype/test/AutoTypeTest.cpp @@ -111,7 +111,7 @@ bool AutoTypePlatformTest::raiseWindow(WId window) } #if defined(Q_OS_MACOS) -bool AutoTypePlatformTest::raiseLastActiveWindow() +bool AutoTypePlatformTest::hideOwnWindow() { return false; } diff --git a/src/autotype/test/AutoTypeTest.h b/src/autotype/test/AutoTypeTest.h index a17028b5..87d19491 100644 --- a/src/autotype/test/AutoTypeTest.h +++ b/src/autotype/test/AutoTypeTest.h @@ -44,7 +44,7 @@ public: AutoTypeExecutor* createExecutor() override; #if defined(Q_OS_MACOS) - bool raiseLastActiveWindow() override; + bool hideOwnWindow() override; bool raiseOwnWindow() override; #endif From 2ffefc95ae22f25d5bac2f60bd486640eff7cea0 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Tue, 9 Apr 2019 21:06:13 -0400 Subject: [PATCH 19/19] Enhance update checker * Reduce initial update check notification to 500 ms to prevent inappropriately stealing focus from user * Add build flag WITH_XC_UPDATECHECK which defaults to ON * Update checks are resolved every 7 days instead of every time the application is started * Better checks for beta builds; ignore snapshots * Increase test cases --- CMakeLists.txt | 5 ++ src/CMakeLists.txt | 1 + src/config-keepassx.h.cmake | 1 + src/core/Clock.cpp | 1 + src/gui/ApplicationSettingsWidget.cpp | 14 +++- src/gui/ApplicationSettingsWidget.h | 1 + src/gui/ApplicationSettingsWidgetGeneral.ui | 39 +++++++-- src/gui/MainWindow.cpp | 12 +-- src/updatecheck/UpdateChecker.cpp | 87 ++++++++++++--------- src/updatecheck/UpdateChecker.h | 2 +- tests/TestUpdateCheck.cpp | 35 +++++++-- 11 files changed, 139 insertions(+), 59 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3e792857..536b08d9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -49,6 +49,7 @@ option(WITH_XC_YUBIKEY "Include YubiKey support." OFF) option(WITH_XC_SSHAGENT "Include SSH agent support." OFF) option(WITH_XC_KEESHARE "Sharing integration with KeeShare" OFF) option(WITH_XC_KEESHARE_SECURE "Sharing integration with secured KeeShare containers" OFF) +option(WITH_XC_UPDATECHECK "Include automatic update checks; disable for controlled distributions" ON) if(APPLE) option(WITH_XC_TOUCHID "Include TouchID support for macOS." OFF) endif() @@ -76,6 +77,10 @@ else() set(WITH_XC_CRYPTO_SSH OFF) endif() +if(WITH_XC_UPDATECHECK) + set(WITH_XC_NETWORKING ON) +endif() + set(KEEPASSXC_VERSION_MAJOR "2") set(KEEPASSXC_VERSION_MINOR "4") set(KEEPASSXC_VERSION_PATCH "0") diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index b2cd2723..d8eb681e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -195,6 +195,7 @@ add_feature_info(SSHAgent WITH_XC_SSHAGENT "SSH agent integration compatible wit add_feature_info(KeeShare WITH_XC_KEESHARE "Sharing integration with KeeShare") add_feature_info(KeeShare-Secure WITH_XC_KEESHARE_SECURE "Sharing integration with KeeShare with secure sources") add_feature_info(YubiKey WITH_XC_YUBIKEY "YubiKey HMAC-SHA1 challenge-response") +add_feature_info(UpdateCheck WITH_XC_UPDATECHECK "Automatic update checking") if(APPLE) add_feature_info(TouchID WITH_XC_TOUCHID "TouchID integration") endif() diff --git a/src/config-keepassx.h.cmake b/src/config-keepassx.h.cmake index 7d701886..2acff446 100644 --- a/src/config-keepassx.h.cmake +++ b/src/config-keepassx.h.cmake @@ -20,6 +20,7 @@ #cmakedefine WITH_XC_KEESHARE #cmakedefine WITH_XC_KEESHARE_INSECURE #cmakedefine WITH_XC_KEESHARE_SECURE +#cmakedefine WITH_XC_UPDATECHECK #cmakedefine WITH_XC_TOUCHID #cmakedefine KEEPASSXC_BUILD_TYPE "@KEEPASSXC_BUILD_TYPE@" diff --git a/src/core/Clock.cpp b/src/core/Clock.cpp index 88ac4fb7..be9e91dc 100644 --- a/src/core/Clock.cpp +++ b/src/core/Clock.cpp @@ -30,6 +30,7 @@ QDateTime Clock::currentDateTime() uint Clock::currentSecondsSinceEpoch() { + // TODO: change to toSecsSinceEpoch() when min Qt >= 5.8 return instance().currentDateTimeImpl().toTime_t(); } diff --git a/src/gui/ApplicationSettingsWidget.cpp b/src/gui/ApplicationSettingsWidget.cpp index 849df03a..22a49dec 100644 --- a/src/gui/ApplicationSettingsWidget.cpp +++ b/src/gui/ApplicationSettingsWidget.cpp @@ -92,8 +92,15 @@ ApplicationSettingsWidget::ApplicationSettingsWidget(QWidget* parent) m_secUi->touchIDResetSpinBox, SLOT(setEnabled(bool))); // clang-format on -#ifndef WITH_XC_NETWORKING +#ifdef WITH_XC_UPDATECHECK + connect(m_generalUi->checkForUpdatesOnStartupCheckBox, SIGNAL(toggled(bool)), SLOT(checkUpdatesToggled(bool))); +#else m_generalUi->checkForUpdatesOnStartupCheckBox->setVisible(false); + m_generalUi->checkForUpdatesIncludeBetasCheckBox->setVisible(false); + m_generalUi->checkUpdatesSpacer->changeSize(0,0, QSizePolicy::Fixed, QSizePolicy::Fixed); +#endif + +#ifndef WITH_XC_NETWORKING m_secUi->privacy->setVisible(false); #endif @@ -350,3 +357,8 @@ void ApplicationSettingsWidget::rememberDatabasesToggled(bool checked) m_generalUi->rememberLastKeyFilesCheckBox->setEnabled(checked); m_generalUi->openPreviousDatabasesOnStartupCheckBox->setEnabled(checked); } + +void ApplicationSettingsWidget::checkUpdatesToggled(bool checked) +{ + m_generalUi->checkForUpdatesIncludeBetasCheckBox->setEnabled(checked); +} diff --git a/src/gui/ApplicationSettingsWidget.h b/src/gui/ApplicationSettingsWidget.h index 85b3b470..dfffbddb 100644 --- a/src/gui/ApplicationSettingsWidget.h +++ b/src/gui/ApplicationSettingsWidget.h @@ -57,6 +57,7 @@ private slots: void systrayToggled(bool checked); void toolbarSettingsToggled(bool checked); void rememberDatabasesToggled(bool checked); + void checkUpdatesToggled(bool checked); private: QWidget* const m_secWidget; diff --git a/src/gui/ApplicationSettingsWidgetGeneral.ui b/src/gui/ApplicationSettingsWidgetGeneral.ui index f84c6a55..9f03bbb5 100644 --- a/src/gui/ApplicationSettingsWidgetGeneral.ui +++ b/src/gui/ApplicationSettingsWidgetGeneral.ui @@ -141,10 +141,40 @@ - Check for updates at application startup + Check for updates at application startup once per week + + + + 0 + + + + + Qt::Horizontal + + + QSizePolicy::Fixed + + + + 20 + 20 + + + + + + + + Include beta releases when checking for updates + + + + + @@ -241,13 +271,6 @@ General - - - - Include pre-releases when checking for updates - - - diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index 9e60b53e..bf8c6654 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -41,7 +41,7 @@ #include "keys/FileKey.h" #include "keys/PasswordKey.h" -#ifdef WITH_XC_NETWORKING +#ifdef WITH_XC_UPDATECHECK #include "gui/MessageBox.h" #include "gui/UpdateCheckDialog.h" #include "updatecheck/UpdateChecker.h" @@ -372,12 +372,12 @@ MainWindow::MainWindow() setUnifiedTitleAndToolBarOnMac(true); #endif -#ifdef WITH_XC_NETWORKING +#ifdef WITH_XC_UPDATECHECK connect(m_ui->actionCheckForUpdates, SIGNAL(triggered()), SLOT(showUpdateCheckDialog())); connect(UpdateChecker::instance(), SIGNAL(updateCheckFinished(bool, QString, bool)), SLOT(hasUpdateAvailable(bool, QString, bool))); - QTimer::singleShot(3000, this, SLOT(showUpdateCheckStartup())); + QTimer::singleShot(500, this, SLOT(showUpdateCheckStartup())); #else m_ui->actionCheckForUpdates->setVisible(false); #endif @@ -670,7 +670,7 @@ void MainWindow::showAboutDialog() void MainWindow::showUpdateCheckStartup() { -#ifdef WITH_XC_NETWORKING +#ifdef WITH_XC_UPDATECHECK if (!config()->get("UpdateCheckMessageShown", false).toBool()) { auto result = MessageBox::question(this, @@ -693,7 +693,7 @@ void MainWindow::showUpdateCheckStartup() void MainWindow::hasUpdateAvailable(bool hasUpdate, const QString& version, bool isManuallyRequested) { -#ifdef WITH_XC_NETWORKING +#ifdef WITH_XC_UPDATECHECK if (hasUpdate && !isManuallyRequested) { auto* updateCheckDialog = new UpdateCheckDialog(this); updateCheckDialog->showUpdateCheckResponse(hasUpdate, version); @@ -708,7 +708,7 @@ void MainWindow::hasUpdateAvailable(bool hasUpdate, const QString& version, bool void MainWindow::showUpdateCheckDialog() { -#ifdef WITH_XC_NETWORKING +#ifdef WITH_XC_UPDATECHECK updateCheck()->checkForUpdates(true); auto* updateCheckDialog = new UpdateCheckDialog(this); updateCheckDialog->show(); diff --git a/src/updatecheck/UpdateChecker.cpp b/src/updatecheck/UpdateChecker.cpp index 4272410b..14531290 100644 --- a/src/updatecheck/UpdateChecker.cpp +++ b/src/updatecheck/UpdateChecker.cpp @@ -17,6 +17,7 @@ #include "UpdateChecker.h" #include "config-keepassx.h" +#include "core/Clock.h" #include "core/Config.h" #include #include @@ -38,24 +39,28 @@ UpdateChecker::~UpdateChecker() void UpdateChecker::checkForUpdates(bool manuallyRequested) { + auto nextCheck = config()->get("GUI/CheckForUpdatesNextCheck", 0).toULongLong(); m_isManuallyRequested = manuallyRequested; - m_bytesReceived.clear(); - QString apiUrlStr = QString("https://api.github.com/repos/keepassxreboot/keepassxc/releases"); + if (m_isManuallyRequested || Clock::currentSecondsSinceEpoch() >= nextCheck) { + m_bytesReceived.clear(); - if (!config()->get("GUI/CheckForUpdatesIncludeBetas", false).toBool()) { - apiUrlStr += "/latest"; + QString apiUrlStr = QString("https://api.github.com/repos/keepassxreboot/keepassxc/releases"); + + if (!config()->get("GUI/CheckForUpdatesIncludeBetas", false).toBool()) { + apiUrlStr += "/latest"; + } + + QUrl apiUrl = QUrl(apiUrlStr); + + QNetworkRequest request(apiUrl); + request.setRawHeader("Accept", "application/json"); + + m_reply = m_netMgr->get(request); + + connect(m_reply, &QNetworkReply::finished, this, &UpdateChecker::fetchFinished); + connect(m_reply, &QIODevice::readyRead, this, &UpdateChecker::fetchReadyRead); } - - QUrl apiUrl = QUrl(apiUrlStr); - - QNetworkRequest request(apiUrl); - request.setRawHeader("Accept", "application/json"); - - m_reply = m_netMgr->get(request); - - connect(m_reply, &QNetworkReply::finished, this, &UpdateChecker::fetchFinished); - connect(m_reply, &QIODevice::readyRead, this, &UpdateChecker::fetchReadyRead); } void UpdateChecker::fetchReadyRead() @@ -84,8 +89,12 @@ void UpdateChecker::fetchFinished() if (!jsonObject.value("tag_name").isUndefined()) { version = jsonObject.value("tag_name").toString(); - hasNewVersion = compareVersions(version, QString(KEEPASSXC_VERSION)); + hasNewVersion = compareVersions(QString(KEEPASSXC_VERSION), version); } + + // Check again in 7 days + // TODO: change to toSecsSinceEpoch() when min Qt >= 5.8 + config()->set("GUI/CheckForUpdatesNextCheck", Clock::currentDateTime().addDays(7).toTime_t()); } else { version = "error"; } @@ -93,38 +102,46 @@ void UpdateChecker::fetchFinished() emit updateCheckFinished(hasNewVersion, version, m_isManuallyRequested); } -bool UpdateChecker::compareVersions(const QString& remoteVersion, const QString& localVersion) +bool UpdateChecker::compareVersions(const QString& localVersion, const QString& remoteVersion) { + // Quick full-string equivalence check if (localVersion == remoteVersion) { - return false; // Currently using updated version + return false; } - QRegularExpression verRegex("^(\\d+(\\.\\d+){0,2})(-\\w+)?$", QRegularExpression::CaseInsensitiveOption); + QRegularExpression verRegex(R"(^((?:\d+\.){2}\d+)(?:-(\w+?)(\d+)?)?$)"); - QRegularExpressionMatch lmatch = verRegex.match(localVersion); - QRegularExpressionMatch rmatch = verRegex.match(remoteVersion); + auto lmatch = verRegex.match(localVersion); + auto rmatch = verRegex.match(remoteVersion); - if (!lmatch.captured(1).isNull() && !rmatch.captured(1).isNull()) { - if (lmatch.captured(1) == rmatch.captured(1) && !lmatch.captured(3).isNull()) { - // Same version, but installed version has snapshot/beta suffix and should be updated to stable - return true; + auto lVersion = lmatch.captured(1).split("."); + auto lSuffix = lmatch.captured(2); + auto lBetaNum = lmatch.captured(3); + + auto rVersion = rmatch.captured(1).split("."); + auto rSuffix = rmatch.captured(2); + auto rBetaNum = rmatch.captured(3); + + if (!lVersion.isEmpty() && !rVersion.isEmpty()) { + if (lSuffix.compare("snapshot", Qt::CaseInsensitive) == 0) { + // Snapshots are not checked for version updates + return false; } - QStringList lparts = lmatch.captured(1).split("."); - QStringList rparts = rmatch.captured(1).split("."); - - if (lparts.length() < 3) - lparts << "0"; - - if (rparts.length() < 3) - rparts << "0"; + // Check "-beta[X]" versions + if (lVersion == rVersion && !lSuffix.isEmpty()) { + // Check if stable version has been released or new beta is available + // otherwise the version numbers are equal + return rSuffix.isEmpty() || lBetaNum.toInt() < rBetaNum.toInt(); + } for (int i = 0; i < 3; i++) { - int l = lparts[i].toInt(); - int r = rparts[i].toInt(); + int l = lVersion[i].toInt(); + int r = rVersion[i].toInt(); - if (l == r) + if (l == r) { continue; + } if (l > r) { return false; // Installed version is newer than release diff --git a/src/updatecheck/UpdateChecker.h b/src/updatecheck/UpdateChecker.h index ac6471d6..64430bda 100644 --- a/src/updatecheck/UpdateChecker.h +++ b/src/updatecheck/UpdateChecker.h @@ -31,7 +31,7 @@ public: ~UpdateChecker() override; void checkForUpdates(bool manuallyRequested); - static bool compareVersions(const QString& remoteVersion, const QString& localVersion); + static bool compareVersions(const QString& localVersion, const QString& remoteVersion); static UpdateChecker* instance(); signals: diff --git a/tests/TestUpdateCheck.cpp b/tests/TestUpdateCheck.cpp index 8cba43b1..ff709cd5 100644 --- a/tests/TestUpdateCheck.cpp +++ b/tests/TestUpdateCheck.cpp @@ -29,13 +29,32 @@ void TestUpdateCheck::initTestCase() void TestUpdateCheck::testCompareVersion() { - // Remote Version , Installed Version - QCOMPARE(UpdateChecker::compareVersions(QString("2.4.0"), QString("2.3.4")), true); - QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.4.0")), false); + // No upgrade QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.0")), false); - QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.0-beta1")), true); - QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta2"), QString("2.3.0-beta1")), true); - QCOMPARE(UpdateChecker::compareVersions(QString("2.3.4"), QString("2.4.0-snapshot")), false); - QCOMPARE(UpdateChecker::compareVersions(QString("invalid"), QString("2.4.0")), false); - QCOMPARE(UpdateChecker::compareVersions(QString(""), QString("2.4.0")), false); + + // First digit upgrade + QCOMPARE(UpdateChecker::compareVersions(QString("2.4.0"), QString("3.0.0")), true); + QCOMPARE(UpdateChecker::compareVersions(QString("3.0.0"), QString("2.4.0")), false); + + // Second digit upgrade + QCOMPARE(UpdateChecker::compareVersions(QString("2.3.4"), QString("2.4.0")), true); + QCOMPARE(UpdateChecker::compareVersions(QString("2.4.0"), QString("2.3.4")), false); + + // Third digit upgrade + QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.1")), true); + QCOMPARE(UpdateChecker::compareVersions(QString("2.3.1"), QString("2.3.0")), false); + + // Beta builds + QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.0-beta1")), false); + QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0"), QString("2.3.1-beta1")), true); + QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta1"), QString("2.3.0")), true); + QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta"), QString("2.3.0-beta1")), true); + QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta1"), QString("2.3.0-beta")), false); + QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta1"), QString("2.3.0-beta2")), true); + QCOMPARE(UpdateChecker::compareVersions(QString("2.3.0-beta2"), QString("2.3.0-beta1")), false); + + // Snapshot and invalid data + QCOMPARE(UpdateChecker::compareVersions(QString("2.3.4-snapshot"), QString("2.4.0")), false); + QCOMPARE(UpdateChecker::compareVersions(QString("2.4.0"), QString("invalid")), false); + QCOMPARE(UpdateChecker::compareVersions(QString("2.4.0"), QString("")), false); }