Welcome to mirror list, hosted at ThFree Co, Russian Federation.

github.com/keepassxreboot/keepassxc.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Klein <42714034+libklein@users.noreply.github.com>2021-11-08 01:41:17 +0300
committerGitHub <noreply@github.com>2021-11-08 01:41:17 +0300
commit84ff6a13f9cd11214396deb31d54955abbcd7b0b (patch)
tree0fa0d84475f9f79c4a8ce12d8582715b0f084940
parent8d7e4918109b1e40660e2988e79e1cf15e899580 (diff)
Allow specifing database backup paths. (#7035)
- Default backupFilePath is '{DB_FILENAME}.old.kdbx' to conform to existing standards - Implement backupPathPattern tests. - Show tooltip on how to format database backup location text field.
-rw-r--r--share/translations/keepassxc_en.ts20
-rw-r--r--src/cli/Add.cpp2
-rw-r--r--src/cli/AddGroup.cpp2
-rw-r--r--src/cli/Create.cpp2
-rw-r--r--src/cli/Edit.cpp2
-rw-r--r--src/cli/Import.cpp2
-rw-r--r--src/cli/Merge.cpp2
-rw-r--r--src/cli/Move.cpp2
-rw-r--r--src/cli/Remove.cpp2
-rw-r--r--src/cli/RemoveGroup.cpp2
-rw-r--r--src/core/Config.cpp6
-rw-r--r--src/core/Config.h2
-rw-r--r--src/core/Database.cpp56
-rw-r--r--src/core/Database.h13
-rw-r--r--src/core/Tools.cpp36
-rw-r--r--src/core/Tools.h2
-rw-r--r--src/gui/ApplicationSettingsWidget.cpp25
-rw-r--r--src/gui/ApplicationSettingsWidget.h1
-rw-r--r--src/gui/ApplicationSettingsWidgetGeneral.ui50
-rw-r--r--src/gui/DatabaseWidget.cpp25
-rw-r--r--tests/TestDatabase.cpp17
-rw-r--r--tests/TestTools.cpp54
-rw-r--r--tests/TestTools.h2
-rw-r--r--tests/gui/TestGui.cpp117
-rw-r--r--tests/gui/TestGui.h5
25 files changed, 368 insertions, 81 deletions
diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts
index dd87e1920..e671d762f 100644
--- a/share/translations/keepassxc_en.ts
+++ b/share/translations/keepassxc_en.ts
@@ -215,6 +215,10 @@
<source>Monochrome</source>
<translation type="unfinished"></translation>
</message>
+ <message>
+ <source>Select backup storage directory</source>
+ <translation type="unfinished"></translation>
+ </message>
</context>
<context>
<name>ApplicationSettingsWidgetGeneral</name>
@@ -440,6 +444,22 @@
<source>Directly write to database file (dangerous)</source>
<translation type="unfinished"></translation>
</message>
+ <message>
+ <source>Choose...</source>
+ <translation type="unfinished"></translation>
+ </message>
+ <message>
+ <source>Backup destination</source>
+ <translation type="unfinished"></translation>
+ </message>
+ <message>
+ <source>Specifies the database backup file location. Occurences of &quot;{DB_FILENAME}&quot; are replaced with the filename of the saved database without extension. {TIME:&lt;format&gt;} is replaced with the backup time, see https://doc.qt.io/qt-5/qdatetime.html#toString. &lt;format&gt; defaults to format string &quot;dd_MM_yyyy_hh-mm-ss&quot;.</source>
+ <translation type="unfinished"></translation>
+ </message>
+ <message>
+ <source>{DB_FILENAME}.old.kdbx</source>
+ <translation type="unfinished"></translation>
+ </message>
</context>
<context>
<name>ApplicationSettingsWidgetSecurity</name>
diff --git a/src/cli/Add.cpp b/src/cli/Add.cpp
index e4bde05f3..409d2a0aa 100644
--- a/src/cli/Add.cpp
+++ b/src/cli/Add.cpp
@@ -121,7 +121,7 @@ int Add::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<Q
}
QString errorMessage;
- if (!database->save(Database::Atomic, false, &errorMessage)) {
+ if (!database->save(Database::Atomic, QString(), &errorMessage)) {
err << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
return EXIT_FAILURE;
}
diff --git a/src/cli/AddGroup.cpp b/src/cli/AddGroup.cpp
index 46837c77b..8fc9ceb39 100644
--- a/src/cli/AddGroup.cpp
+++ b/src/cli/AddGroup.cpp
@@ -63,7 +63,7 @@ int AddGroup::executeWithDatabase(QSharedPointer<Database> database, QSharedPoin
newGroup->setParent(parentGroup);
QString errorMessage;
- if (!database->save(Database::Atomic, false, &errorMessage)) {
+ if (!database->save(Database::Atomic, QString(), &errorMessage)) {
err << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
return EXIT_FAILURE;
}
diff --git a/src/cli/Create.cpp b/src/cli/Create.cpp
index 867471697..4f820d000 100644
--- a/src/cli/Create.cpp
+++ b/src/cli/Create.cpp
@@ -165,7 +165,7 @@ int Create::execute(const QStringList& arguments)
}
QString errorMessage;
- if (!db->saveAs(databaseFilename, Database::Atomic, false, &errorMessage)) {
+ if (!db->saveAs(databaseFilename, Database::Atomic, QString(), &errorMessage)) {
err << QObject::tr("Failed to save the database: %1.").arg(errorMessage) << endl;
return EXIT_FAILURE;
}
diff --git a/src/cli/Edit.cpp b/src/cli/Edit.cpp
index 8941df5c5..ad1d701f7 100644
--- a/src/cli/Edit.cpp
+++ b/src/cli/Edit.cpp
@@ -126,7 +126,7 @@ int Edit::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
entry->endUpdate();
QString errorMessage;
- if (!database->save(Database::Atomic, false, &errorMessage)) {
+ if (!database->save(Database::Atomic, QString(), &errorMessage)) {
err << QObject::tr("Writing the database failed: %1").arg(errorMessage) << endl;
return EXIT_FAILURE;
}
diff --git a/src/cli/Import.cpp b/src/cli/Import.cpp
index 4c1e99437..432507d20 100644
--- a/src/cli/Import.cpp
+++ b/src/cli/Import.cpp
@@ -75,7 +75,7 @@ int Import::execute(const QStringList& arguments)
return EXIT_FAILURE;
}
- if (!db->saveAs(dbPath, Database::Atomic, false, &errorMessage)) {
+ if (!db->saveAs(dbPath, Database::Atomic, QString(), &errorMessage)) {
err << QObject::tr("Failed to save the database: %1.").arg(errorMessage) << endl;
return EXIT_FAILURE;
}
diff --git a/src/cli/Merge.cpp b/src/cli/Merge.cpp
index 41e18ca0b..a80f454bb 100644
--- a/src/cli/Merge.cpp
+++ b/src/cli/Merge.cpp
@@ -95,7 +95,7 @@ int Merge::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer
if (!changeList.isEmpty() && !parser->isSet(Merge::DryRunOption)) {
QString errorMessage;
- if (!database->save(Database::Atomic, false, &errorMessage)) {
+ if (!database->save(Database::Atomic, QString(), &errorMessage)) {
err << QObject::tr("Unable to save database to file : %1").arg(errorMessage) << endl;
return EXIT_FAILURE;
}
diff --git a/src/cli/Move.cpp b/src/cli/Move.cpp
index 350cc4b82..990f45691 100644
--- a/src/cli/Move.cpp
+++ b/src/cli/Move.cpp
@@ -65,7 +65,7 @@ int Move::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
entry->endUpdate();
QString errorMessage;
- if (!database->save(Database::Atomic, false, &errorMessage)) {
+ if (!database->save(Database::Atomic, QString(), &errorMessage)) {
err << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
return EXIT_FAILURE;
}
diff --git a/src/cli/Remove.cpp b/src/cli/Remove.cpp
index 6ed667fa1..ed6a27502 100644
--- a/src/cli/Remove.cpp
+++ b/src/cli/Remove.cpp
@@ -53,7 +53,7 @@ int Remove::executeWithDatabase(QSharedPointer<Database> database, QSharedPointe
};
QString errorMessage;
- if (!database->save(Database::Atomic, false, &errorMessage)) {
+ if (!database->save(Database::Atomic, QString(), &errorMessage)) {
err << QObject::tr("Unable to save database to file: %1").arg(errorMessage) << endl;
return EXIT_FAILURE;
}
diff --git a/src/cli/RemoveGroup.cpp b/src/cli/RemoveGroup.cpp
index e1d1d7e69..df6bf4cfa 100644
--- a/src/cli/RemoveGroup.cpp
+++ b/src/cli/RemoveGroup.cpp
@@ -63,7 +63,7 @@ int RemoveGroup::executeWithDatabase(QSharedPointer<Database> database, QSharedP
};
QString errorMessage;
- if (!database->save(Database::Atomic, false, &errorMessage)) {
+ if (!database->save(Database::Atomic, QString(), &errorMessage)) {
err << QObject::tr("Unable to save database to file: %1").arg(errorMessage) << endl;
return EXIT_FAILURE;
}
diff --git a/src/core/Config.cpp b/src/core/Config.cpp
index 54fc59c81..a6d179504 100644
--- a/src/core/Config.cpp
+++ b/src/core/Config.cpp
@@ -61,6 +61,7 @@ static const QHash<Config::ConfigKey, ConfigDirective> configStrings = {
{Config::AutoSaveOnExit,{QS("AutoSaveOnExit"), Roaming, true}},
{Config::AutoSaveNonDataChanges,{QS("AutoSaveNonDataChanges"), Roaming, true}},
{Config::BackupBeforeSave,{QS("BackupBeforeSave"), Roaming, false}},
+ {Config::BackupFilePathPattern,{QS("BackupFilePathPattern"), Roaming, QString("{DB_FILENAME}.old.kdbx")}},
{Config::UseAtomicSaves,{QS("UseAtomicSaves"), Roaming, true}},
{Config::UseDirectWriteSaves,{QS("UseDirectWriteSaves"), Local, false}},
{Config::SearchLimitGroup,{QS("SearchLimitGroup"), Roaming, false}},
@@ -229,6 +230,11 @@ QVariant Config::get(ConfigKey key)
return m_settings->value(cfg.name, defaultValue);
}
+QVariant Config::getDefault(Config::ConfigKey key)
+{
+ return configStrings[key].defaultValue;
+}
+
bool Config::hasAccessError()
{
return m_settings->status() & QSettings::AccessError;
diff --git a/src/core/Config.h b/src/core/Config.h
index 8e5c14e35..1be8699ca 100644
--- a/src/core/Config.h
+++ b/src/core/Config.h
@@ -43,6 +43,7 @@ public:
AutoSaveOnExit,
AutoSaveNonDataChanges,
BackupBeforeSave,
+ BackupFilePathPattern,
UseAtomicSaves,
UseDirectWriteSaves,
SearchLimitGroup,
@@ -195,6 +196,7 @@ public:
~Config() override;
QVariant get(ConfigKey key);
+ QVariant getDefault(ConfigKey key);
QString getFileName();
void set(ConfigKey key, const QVariant& value);
void remove(ConfigKey key);
diff --git a/src/core/Database.cpp b/src/core/Database.cpp
index b30c3623a..5738a3b0a 100644
--- a/src/core/Database.cpp
+++ b/src/core/Database.cpp
@@ -178,10 +178,10 @@ bool Database::isSaving()
*
* @param error error message in case of failure
* @param atomic Use atomic file transactions
- * @param backup Backup the existing database file, if exists
+ * @param backupFilePath Absolute file path to write the backup file to. Pass an empty QString to disable backup.
* @return true on success
*/
-bool Database::save(SaveAction action, bool backup, QString* error)
+bool Database::save(SaveAction action, const QString& backupFilePath, QString* error)
{
Q_ASSERT(!m_data.filePath.isEmpty());
if (m_data.filePath.isEmpty()) {
@@ -191,7 +191,7 @@ bool Database::save(SaveAction action, bool backup, QString* error)
return false;
}
- return saveAs(m_data.filePath, action, backup, error);
+ return saveAs(m_data.filePath, action, backupFilePath, error);
}
/**
@@ -209,10 +209,11 @@ bool Database::save(SaveAction action, bool backup, QString* error)
* @param filePath Absolute path of the file to save
* @param error error message in case of failure
* @param atomic Use atomic file transactions
- * @param backup Backup the existing database file, if exists
+ * @param backupFilePath Absolute path to the location where the backup should be stored. Passing an empty string
+ * disables backup.
* @return true on success
*/
-bool Database::saveAs(const QString& filePath, SaveAction action, bool backup, QString* error)
+bool Database::saveAs(const QString& filePath, SaveAction action, const QString& backupFilePath, QString* error)
{
// Disallow overlapping save operations
if (isSaving()) {
@@ -260,7 +261,7 @@ bool Database::saveAs(const QString& filePath, SaveAction action, bool backup, Q
QFileInfo fileInfo(filePath);
auto realFilePath = fileInfo.exists() ? fileInfo.canonicalFilePath() : fileInfo.absoluteFilePath();
bool isNewFile = !QFile::exists(realFilePath);
- bool ok = AsyncTask::runAndWaitForFuture([&] { return performSave(realFilePath, action, backup, error); });
+ bool ok = AsyncTask::runAndWaitForFuture([&] { return performSave(realFilePath, action, backupFilePath, error); });
if (ok) {
markAsClean();
setFilePath(filePath);
@@ -276,10 +277,10 @@ bool Database::saveAs(const QString& filePath, SaveAction action, bool backup, Q
return ok;
}
-bool Database::performSave(const QString& filePath, SaveAction action, bool backup, QString* error)
+bool Database::performSave(const QString& filePath, SaveAction action, const QString& backupFilePath, QString* error)
{
- if (backup) {
- backupDatabase(filePath);
+ if (!backupFilePath.isNull()) {
+ backupDatabase(filePath, backupFilePath);
}
#if QT_VERSION >= QT_VERSION_CHECK(5, 10, 0)
@@ -337,7 +338,7 @@ bool Database::performSave(const QString& filePath, SaveAction action, bool back
tempFile.setFileTime(createTime, QFile::FileBirthTime);
#endif
return true;
- } else if (!backup || !restoreDatabase(filePath)) {
+ } else if (backupFilePath.isEmpty() || !restoreDatabase(filePath, backupFilePath)) {
// Failed to copy new database in place, and
// failed to restore from backup or backups disabled
tempFile.setAutoRemove(false);
@@ -485,23 +486,26 @@ void Database::releaseData()
}
/**
- * Remove the old backup and replace it with a new one
- * backups are named <filename>.old.<extension>
+ * Remove the old backup and replace it with a new one. Backup name is taken from destinationFilePath.
+ * Non-existing parent directories will be created automatically.
*
* @param filePath Path to the file to backup
+ * @param destinationFilePath Path to the backup destination file
* @return true on success
*/
-bool Database::backupDatabase(const QString& filePath)
+bool Database::backupDatabase(const QString& filePath, const QString& destinationFilePath)
{
- static auto re = QRegularExpression("(\\.[^.]+)$");
-
- auto match = re.match(filePath);
- auto backupFilePath = filePath;
+ // Ensure that the path to write to actually exists
+ auto parentDirectory = QFileInfo(destinationFilePath).absoluteDir();
+ if (!parentDirectory.exists()) {
+ if (!QDir().mkpath(parentDirectory.absolutePath())) {
+ return false;
+ }
+ }
auto perms = QFile::permissions(filePath);
- backupFilePath = backupFilePath.replace(re, "") + ".old" + match.captured(1);
- QFile::remove(backupFilePath);
- bool res = QFile::copy(filePath, backupFilePath);
- QFile::setPermissions(backupFilePath, perms);
+ QFile::remove(destinationFilePath);
+ bool res = QFile::copy(filePath, destinationFilePath);
+ QFile::setPermissions(destinationFilePath, perms);
return res;
}
@@ -513,17 +517,13 @@ bool Database::backupDatabase(const QString& filePath)
* @param filePath Path to the file to restore
* @return true on success
*/
-bool Database::restoreDatabase(const QString& filePath)
+bool Database::restoreDatabase(const QString& filePath, const QString& fromBackupFilePath)
{
- static auto re = QRegularExpression("^(.*?)(\\.[^.]+)?$");
-
- auto match = re.match(filePath);
auto perms = QFile::permissions(filePath);
- auto backupFilePath = match.captured(1) + ".old" + match.captured(2);
// Only try to restore if the backup file actually exists
- if (QFile::exists(backupFilePath)) {
+ if (QFile::exists(fromBackupFilePath)) {
QFile::remove(filePath);
- if (QFile::copy(backupFilePath, filePath)) {
+ if (QFile::copy(fromBackupFilePath, filePath)) {
return QFile::setPermissions(filePath, perms);
}
}
diff --git a/src/core/Database.h b/src/core/Database.h
index 24bb79db2..c42025f85 100644
--- a/src/core/Database.h
+++ b/src/core/Database.h
@@ -79,8 +79,11 @@ public:
QSharedPointer<const CompositeKey> key,
QString* error = nullptr,
bool readOnly = false);
- bool save(SaveAction action = Atomic, bool backup = false, QString* error = nullptr);
- bool saveAs(const QString& filePath, SaveAction action = Atomic, bool backup = false, QString* error = nullptr);
+ bool save(SaveAction action = Atomic, const QString& backupFilePath = QString(), QString* error = nullptr);
+ bool saveAs(const QString& filePath,
+ SaveAction action = Atomic,
+ const QString& backupFilePath = QString(),
+ QString* error = nullptr);
bool extract(QByteArray&, QString* error = nullptr);
bool import(const QString& xmlExportPath, QString* error = nullptr);
@@ -203,9 +206,9 @@ private:
void createRecycleBin();
bool writeDatabase(QIODevice* device, QString* error = nullptr);
- bool backupDatabase(const QString& filePath);
- bool restoreDatabase(const QString& filePath);
- bool performSave(const QString& filePath, SaveAction flags, bool backup, QString* error);
+ bool backupDatabase(const QString& filePath, const QString& destinationFilePath);
+ bool restoreDatabase(const QString& filePath, const QString& fromBackupFilePath);
+ bool performSave(const QString& filePath, SaveAction flags, const QString& backupFilePath, QString* error);
void startModifiedTimer();
void stopModifiedTimer();
diff --git a/src/core/Tools.cpp b/src/core/Tools.cpp
index 58a38f433..7c6d52a59 100644
--- a/src/core/Tools.cpp
+++ b/src/core/Tools.cpp
@@ -22,7 +22,10 @@
#include "config-keepassx.h"
#include "git-info.h"
+#include "core/Clock.h"
+
#include <QElapsedTimer>
+#include <QFileInfo>
#include <QImageReader>
#include <QLocale>
#include <QMetaProperty>
@@ -376,4 +379,37 @@ namespace Tools
}
return result;
}
+
+ QString substituteBackupFilePath(QString pattern, const QString& databasePath)
+ {
+ // Fail if substitution fails
+ if (databasePath.isEmpty()) {
+ return {};
+ }
+
+ // Replace backup pattern
+ QFileInfo dbFileInfo(databasePath);
+ QString baseName = dbFileInfo.completeBaseName();
+
+ pattern.replace(QString("{DB_FILENAME}"), baseName);
+
+ auto re = QRegularExpression(R"(\{TIME(?::([^\\]*))?\})");
+ auto match = re.match(pattern);
+ while (match.hasMatch()) {
+ // Extract time format specifier
+ auto formatSpecifier = QString("dd_MM_yyyy_hh-mm-ss");
+ if (!match.captured(1).isEmpty()) {
+ formatSpecifier = match.captured(1);
+ }
+ auto replacement = Clock::currentDateTime().toString(formatSpecifier);
+ pattern.replace(match.capturedStart(), match.capturedLength(), replacement);
+ match = re.match(pattern);
+ }
+
+ // Replace escaped braces
+ pattern.replace("\\{", "{");
+ pattern.replace("\\}", "}");
+
+ return pattern;
+ }
} // namespace Tools
diff --git a/src/core/Tools.h b/src/core/Tools.h
index 8ebcef1c5..cf3b3593d 100644
--- a/src/core/Tools.h
+++ b/src/core/Tools.h
@@ -75,6 +75,8 @@ namespace Tools
}
QVariantMap qo2qvm(const QObject* object, const QStringList& ignoredProperties = {"objectName"});
+
+ QString substituteBackupFilePath(QString pattern, const QString& databasePath);
} // namespace Tools
#endif // KEEPASSX_TOOLS_H
diff --git a/src/gui/ApplicationSettingsWidget.cpp b/src/gui/ApplicationSettingsWidget.cpp
index 100258120..ac96d23b4 100644
--- a/src/gui/ApplicationSettingsWidget.cpp
+++ b/src/gui/ApplicationSettingsWidget.cpp
@@ -19,6 +19,8 @@
#include "ApplicationSettingsWidget.h"
#include "ui_ApplicationSettingsWidgetGeneral.h"
#include "ui_ApplicationSettingsWidgetSecurity.h"
+#include <QDesktopServices>
+#include <QDir>
#include "config-keepassx.h"
@@ -28,6 +30,7 @@
#include "gui/MainWindow.h"
#include "gui/osutils/OSUtils.h"
+#include "FileDialog.h"
#include "MessageBox.h"
#ifdef Q_OS_MACOS
#include "touchid/TouchID.h"
@@ -112,6 +115,12 @@ ApplicationSettingsWidget::ApplicationSettingsWidget(QWidget* parent)
connect(m_generalUi->useAlternativeSaveCheckBox, SIGNAL(toggled(bool)),
m_generalUi->alternativeSaveComboBox, SLOT(setEnabled(bool)));
+ connect(m_generalUi->backupBeforeSaveCheckBox, SIGNAL(toggled(bool)),
+ m_generalUi->backupFilePath, SLOT(setEnabled(bool)));
+ connect(m_generalUi->backupBeforeSaveCheckBox, SIGNAL(toggled(bool)),
+ m_generalUi->backupFilePathPicker, SLOT(setEnabled(bool)));
+ connect(m_generalUi->backupFilePathPicker, SIGNAL(pressed()), SLOT(selectBackupDirectory()));
+
connect(m_secUi->clearClipboardCheckBox, SIGNAL(toggled(bool)),
m_secUi->clearClipboardSpinBox, SLOT(setEnabled(bool)));
connect(m_secUi->clearSearchCheckBox, SIGNAL(toggled(bool)),
@@ -188,6 +197,9 @@ void ApplicationSettingsWidget::loadSettings()
m_generalUi->autoSaveOnExitCheckBox->setChecked(config()->get(Config::AutoSaveOnExit).toBool());
m_generalUi->autoSaveNonDataChangesCheckBox->setChecked(config()->get(Config::AutoSaveNonDataChanges).toBool());
m_generalUi->backupBeforeSaveCheckBox->setChecked(config()->get(Config::BackupBeforeSave).toBool());
+
+ m_generalUi->backupFilePath->setText(config()->get(Config::BackupFilePathPattern).toString());
+
m_generalUi->useAlternativeSaveCheckBox->setChecked(!config()->get(Config::UseAtomicSaves).toBool());
m_generalUi->alternativeSaveComboBox->setCurrentIndex(config()->get(Config::UseDirectWriteSaves).toBool() ? 1 : 0);
m_generalUi->autoReloadOnChangeCheckBox->setChecked(config()->get(Config::AutoReloadOnChange).toBool());
@@ -326,6 +338,9 @@ void ApplicationSettingsWidget::saveSettings()
config()->set(Config::AutoSaveOnExit, m_generalUi->autoSaveOnExitCheckBox->isChecked());
config()->set(Config::AutoSaveNonDataChanges, m_generalUi->autoSaveNonDataChangesCheckBox->isChecked());
config()->set(Config::BackupBeforeSave, m_generalUi->backupBeforeSaveCheckBox->isChecked());
+
+ config()->set(Config::BackupFilePathPattern, m_generalUi->backupFilePath->text());
+
config()->set(Config::UseAtomicSaves, !m_generalUi->useAlternativeSaveCheckBox->isChecked());
config()->set(Config::UseDirectWriteSaves, m_generalUi->alternativeSaveComboBox->currentIndex() == 1);
config()->set(Config::AutoReloadOnChange, m_generalUi->autoReloadOnChangeCheckBox->isChecked());
@@ -504,3 +519,13 @@ void ApplicationSettingsWidget::checkUpdatesToggled(bool checked)
{
m_generalUi->checkForUpdatesIncludeBetasCheckBox->setEnabled(checked);
}
+
+void ApplicationSettingsWidget::selectBackupDirectory()
+{
+ auto backupDirectory =
+ FileDialog::instance()->getExistingDirectory(this, tr("Select backup storage directory"), QDir::homePath());
+ if (!backupDirectory.isEmpty()) {
+ m_generalUi->backupFilePath->setText(
+ QDir(backupDirectory).filePath(config()->getDefault(Config::BackupFilePathPattern).toString()));
+ }
+} \ No newline at end of file
diff --git a/src/gui/ApplicationSettingsWidget.h b/src/gui/ApplicationSettingsWidget.h
index f36e5ef12..c5f2ed7e3 100644
--- a/src/gui/ApplicationSettingsWidget.h
+++ b/src/gui/ApplicationSettingsWidget.h
@@ -62,6 +62,7 @@ private slots:
void systrayToggled(bool checked);
void rememberDatabasesToggled(bool checked);
void checkUpdatesToggled(bool checked);
+ void selectBackupDirectory();
private:
QWidget* const m_secWidget;
diff --git a/src/gui/ApplicationSettingsWidgetGeneral.ui b/src/gui/ApplicationSettingsWidgetGeneral.ui
index 0e18dfde6..7729e7c8f 100644
--- a/src/gui/ApplicationSettingsWidgetGeneral.ui
+++ b/src/gui/ApplicationSettingsWidgetGeneral.ui
@@ -58,8 +58,8 @@
<rect>
<x>0</x>
<y>0</y>
- <width>581</width>
- <height>924</height>
+ <width>664</width>
+ <height>1215</height>
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout_8">
@@ -279,6 +279,52 @@
</widget>
</item>
<item>
+ <layout class="QHBoxLayout" name="horizontalLayout_4">
+ <item>
+ <widget class="QLabel" name="label">
+ <property name="sizePolicy">
+ <sizepolicy hsizetype="Minimum" vsizetype="Preferred">
+ <horstretch>0</horstretch>
+ <verstretch>0</verstretch>
+ </sizepolicy>
+ </property>
+ <property name="text">
+ <string>Backup destination</string>
+ </property>
+ <property name="buddy">
+ <cstring>backupFilePath</cstring>
+ </property>
+ </widget>
+ </item>
+ <item>
+ <widget class="QLineEdit" name="backupFilePath">
+ <property name="enabled">
+ <bool>false</bool>
+ </property>
+ <property name="toolTip">
+ <string>Specifies the database backup file location. Occurences of &quot;{DB_FILENAME}&quot; are replaced with the filename of the saved database without extension. {TIME:&lt;format&gt;} is replaced with the backup time, see https://doc.qt.io/qt-5/qdatetime.html#toString. &lt;format&gt; defaults to format string &quot;dd_MM_yyyy_hh-mm-ss&quot;.</string>
+ </property>
+ <property name="placeholderText">
+ <string>{DB_FILENAME}.old.kdbx</string>
+ </property>
+ </widget>
+ </item>
+ <item>
+ <widget class="QPushButton" name="backupFilePathPicker">
+ <property name="enabled">
+ <bool>false</bool>
+ </property>
+ <property name="text">
+ <string>Choose...</string>
+ </property>
+ </widget>
+ </item>
+ </layout>
+ </item>
+ <item>
+ <layout class="QHBoxLayout" name="horizontalLayout_5"/>
+ </item>
+ <item>
<widget class="QCheckBox" name="useAlternativeSaveCheckBox">
<property name="text">
<string>Use alternative saving method (may solve problems with Dropbox, Google Drive, GVFS, etc.)</string>
diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp
index 7a7d961ae..6d88a6613 100644
--- a/src/gui/DatabaseWidget.cpp
+++ b/src/gui/DatabaseWidget.cpp
@@ -27,6 +27,7 @@
#include <QProcess>
#include <QSplitter>
#include <QTextEdit>
+#include <core/Tools.h>
#include "autotype/AutoType.h"
#include "core/EntrySearcher.h"
@@ -1879,11 +1880,31 @@ bool DatabaseWidget::performSave(QString& errorMessage, const QString& fileName)
}
}
+ QString backupFilePath;
+ if (config()->get(Config::BackupBeforeSave).toBool()) {
+ backupFilePath = config()->get(Config::BackupFilePathPattern).toString();
+ // Fall back to default
+ if (backupFilePath.isEmpty()) {
+ backupFilePath = config()->getDefault(Config::BackupFilePathPattern).toString();
+ }
+
+ QFileInfo dbFileInfo(m_db->filePath());
+ backupFilePath = Tools::substituteBackupFilePath(backupFilePath, dbFileInfo.canonicalFilePath());
+ if (!backupFilePath.isNull()) {
+ // Note that we cannot guarantee that backupFilePath is actually a valid filename. QT currently provides
+ // no function for this. Moreover, we don't check if backupFilePath is a file and not a directory.
+ // If this isn't the case, just let the backup fail.
+ if (QDir::isRelativePath(backupFilePath)) {
+ backupFilePath = QDir::cleanPath(dbFileInfo.absolutePath() + QDir::separator() + backupFilePath);
+ }
+ }
+ }
+
bool ok;
if (fileName.isEmpty()) {
- ok = m_db->save(saveAction, config()->get(Config::BackupBeforeSave).toBool(), &errorMessage);
+ ok = m_db->save(saveAction, backupFilePath, &errorMessage);
} else {
- ok = m_db->saveAs(fileName, saveAction, config()->get(Config::BackupBeforeSave).toBool(), &errorMessage);
+ ok = m_db->saveAs(fileName, saveAction, backupFilePath, &errorMessage);
}
// Return control
diff --git a/tests/TestDatabase.cpp b/tests/TestDatabase.cpp
index 989d4957c..9fa09d315 100644
--- a/tests/TestDatabase.cpp
+++ b/tests/TestDatabase.cpp
@@ -75,29 +75,26 @@ void TestDatabase::testSave()
// Test safe saves
db->metadata()->setName("test");
QVERIFY(db->isModified());
- QVERIFY2(db->save(Database::Atomic, false, &error), error.toLatin1());
+ QVERIFY2(db->save(Database::Atomic, QString(), &error), error.toLatin1());
QVERIFY(!db->isModified());
// Test temp-file saves
db->metadata()->setName("test2");
- QVERIFY2(db->save(Database::TempFile, false, &error), error.toLatin1());
+ QVERIFY2(db->save(Database::TempFile, QString(), &error), error.toLatin1());
QVERIFY(!db->isModified());
// Test direct-write saves
db->metadata()->setName("test3");
- QVERIFY2(db->save(Database::DirectWrite, false, &error), error.toLatin1());
+ QVERIFY2(db->save(Database::DirectWrite, QString(), &error), error.toLatin1());
QVERIFY(!db->isModified());
// Test save backups
+ TemporaryFile backupFile;
+ auto backupFilePath = backupFile.fileName();
db->metadata()->setName("test4");
- QVERIFY2(db->save(Database::Atomic, true, &error), error.toLatin1());
+ QVERIFY2(db->save(Database::Atomic, backupFilePath, &error), error.toLatin1());
QVERIFY(!db->isModified());
- // Confirm backup exists and then delete it
- auto re = QRegularExpression("(\\.[^.]+)$");
- auto match = re.match(tempFile.fileName());
- auto backupFilePath = tempFile.fileName();
- backupFilePath = backupFilePath.replace(re, "") + ".old" + match.captured(1);
QVERIFY(QFile::exists(backupFilePath));
QFile::remove(backupFilePath);
QVERIFY(!QFile::exists(backupFilePath));
@@ -123,7 +120,7 @@ void TestDatabase::testSignals()
QTRY_COMPARE(spyModified.count(), 1);
QSignalSpy spySaved(db.data(), SIGNAL(databaseSaved()));
- QVERIFY(db->save(Database::Atomic, false, &error));
+ QVERIFY(db->save(Database::Atomic, QString(), &error));
QCOMPARE(spySaved.count(), 1);
// Short delay to allow file system settling to reduce test failures
diff --git a/tests/TestTools.cpp b/tests/TestTools.cpp
index 881434c19..edfeb5034 100644
--- a/tests/TestTools.cpp
+++ b/tests/TestTools.cpp
@@ -17,6 +17,8 @@
#include "TestTools.h"
+#include "core/Clock.h"
+
#include <QTest>
QTEST_GUILESS_MAIN(TestTools)
@@ -108,3 +110,55 @@ void TestTools::testValidUuid()
QVERIFY(!Tools::isValidUuid(longUuid));
QVERIFY(!Tools::isValidUuid(nonHexUuid));
}
+
+void TestTools::testBackupFilePatternSubstitution_data()
+{
+ QTest::addColumn<QString>("pattern");
+ QTest::addColumn<QString>("dbFilePath");
+ QTest::addColumn<QString>("expectedSubstitution");
+
+ static const auto DEFAULT_DB_FILE_NAME = QStringLiteral("KeePassXC");
+ static const auto DEFAULT_DB_FILE_PATH = QStringLiteral("/tmp/") + DEFAULT_DB_FILE_NAME + QStringLiteral(".kdbx");
+ static const auto NOW = Clock::currentDateTime();
+ auto DEFAULT_FORMATTED_TIME = NOW.toString("dd_MM_yyyy_hh-mm-ss");
+
+ QTest::newRow("Null pattern") << QString() << DEFAULT_DB_FILE_PATH << QString();
+ QTest::newRow("Empty pattern") << QString("") << DEFAULT_DB_FILE_PATH << QString("");
+ QTest::newRow("Null database path") << "valid_pattern" << QString() << QString();
+ QTest::newRow("Empty database path") << "valid_pattern" << QString("") << QString();
+ QTest::newRow("Unclosed/invalid pattern") << "{DB_FILENAME" << DEFAULT_DB_FILE_PATH << "{DB_FILENAME";
+ QTest::newRow("Unknown pattern") << "{NO_MATCH}" << DEFAULT_DB_FILE_PATH << "{NO_MATCH}";
+ QTest::newRow("Do not replace escaped patterns (filename)")
+ << "\\{DB_FILENAME\\}" << DEFAULT_DB_FILE_PATH << "{DB_FILENAME}";
+ QTest::newRow("Do not replace escaped patterns (time)")
+ << "\\{TIME:dd.MM.yyyy\\}" << DEFAULT_DB_FILE_PATH << "{TIME:dd.MM.yyyy}";
+ QTest::newRow("Multiple patterns should be replaced")
+ << "{DB_FILENAME} {TIME} {DB_FILENAME}" << DEFAULT_DB_FILE_PATH
+ << DEFAULT_DB_FILE_NAME + QStringLiteral(" ") + DEFAULT_FORMATTED_TIME + QStringLiteral(" ")
+ + DEFAULT_DB_FILE_NAME;
+ QTest::newRow("Default time pattern") << "{TIME}" << DEFAULT_DB_FILE_PATH << DEFAULT_FORMATTED_TIME;
+ QTest::newRow("Default time pattern (empty formatter)")
+ << "{TIME:}" << DEFAULT_DB_FILE_PATH << DEFAULT_FORMATTED_TIME;
+ QTest::newRow("Custom time pattern") << "{TIME:dd-ss}" << DEFAULT_DB_FILE_PATH << NOW.toString("dd-ss");
+ QTest::newRow("Invalid custom time pattern") << "{TIME:dd/-ss}" << DEFAULT_DB_FILE_PATH << NOW.toString("dd/-ss");
+ QTest::newRow("Recursive substitution") << "{TIME:'{TIME}'}" << DEFAULT_DB_FILE_PATH << DEFAULT_FORMATTED_TIME;
+ QTest::newRow("{DB_FILENAME} substitution")
+ << "some {DB_FILENAME} thing" << DEFAULT_DB_FILE_PATH
+ << QStringLiteral("some ") + DEFAULT_DB_FILE_NAME + QStringLiteral(" thing");
+ QTest::newRow("{DB_FILENAME} substitution with multiple extensions") << "some {DB_FILENAME} thing"
+ << "/tmp/KeePassXC.kdbx.ext"
+ << "some KeePassXC.kdbx thing";
+ // Not relevant right now, added test anyway
+ QTest::newRow("There should be no substitution loops") << "{DB_FILENAME}"
+ << "{TIME:'{DB_FILENAME}'}.ext"
+ << "{DB_FILENAME}";
+}
+
+void TestTools::testBackupFilePatternSubstitution()
+{
+ QFETCH(QString, pattern);
+ QFETCH(QString, dbFilePath);
+ QFETCH(QString, expectedSubstitution);
+
+ QCOMPARE(Tools::substituteBackupFilePath(pattern, dbFilePath), expectedSubstitution);
+}
diff --git a/tests/TestTools.h b/tests/TestTools.h
index 24554b6fd..29f2bf0f5 100644
--- a/tests/TestTools.h
+++ b/tests/TestTools.h
@@ -29,6 +29,8 @@ private slots:
void testIsBase64();
void testEnvSubstitute();
void testValidUuid();
+ void testBackupFilePatternSubstitution_data();
+ void testBackupFilePatternSubstitution();
};
#endif // KEEPASSX_TESTTOOLS_H
diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp
index 2a669c41d..e6c0c3e12 100644
--- a/tests/gui/TestGui.cpp
+++ b/tests/gui/TestGui.cpp
@@ -82,22 +82,10 @@ int main(int argc, char* argv[])
return QTest::qExec(&tc, argc, argv);
}
-static QString dbFileName = QStringLiteral(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx");
-
void TestGui::initTestCase()
{
QVERIFY(Crypto::init());
Config::createTempFileInstance();
- // Disable autosave so we can test the modified file indicator
- config()->set(Config::AutoSaveAfterEveryChange, false);
- config()->set(Config::AutoSaveOnExit, false);
- // Enable the tray icon so we can test hiding/restoring the windowQByteArray
- config()->set(Config::GUI_ShowTrayIcon, true);
- // Disable advanced settings mode (activate within individual tests to test advanced settings)
- config()->set(Config::GUI_AdvancedSettings, false);
- // Disable the update check first time alert
- config()->set(Config::UpdateCheckMessageShown, true);
-
Application::bootstrap();
m_mainWindow.reset(new MainWindow());
@@ -106,11 +94,22 @@ void TestGui::initTestCase()
m_mainWindow->resize(1024, 768);
}
-// Every test starts with opening the temp database
+// Every test starts with resetting config settings and opening the temp database
void TestGui::init()
{
+ // Reset config to defaults
+ config()->resetToDefaults();
+ // Disable autosave so we can test the modified file indicator
+ config()->set(Config::AutoSaveAfterEveryChange, false);
+ config()->set(Config::AutoSaveOnExit, false);
+ // Enable the tray icon so we can test hiding/restoring the windowQByteArray
+ config()->set(Config::GUI_ShowTrayIcon, true);
+ // Disable the update check first time alert
+ config()->set(Config::UpdateCheckMessageShown, true);
+
// Copy the test database file to the temporary file
- QVERIFY(m_dbFile.copyFromFile(dbFileName));
+ auto origFilePath = QDir(KEEPASSX_TEST_DATA_DIR).absoluteFilePath("NewDatabase.kdbx");
+ QVERIFY(m_dbFile.copyFromFile(origFilePath));
m_dbFileName = QFileInfo(m_dbFile.fileName()).fileName();
m_dbFilePath = m_dbFile.fileName();
@@ -354,6 +353,8 @@ void TestGui::testAutoreloadDatabase()
cleanup();
init();
+ config()->set(Config::AutoReloadOnChange, false);
+
// Test rejecting new file in autoreload
MessageBox::setNextAnswer(MessageBox::No);
// Overwrite the current database with the temp data
@@ -1277,11 +1278,76 @@ void TestGui::testSave()
QTRY_COMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("testSave*"));
triggerAction("actionDatabaseSave");
- QCOMPARE(m_tabWidget->tabName(m_tabWidget->currentIndex()), QString("testSave"));
+ QTRY_COMPARE(m_tabWidget->tabName(m_tabWidget->currentIndex()), QString("testSave"));
checkDatabase();
}
+void TestGui::testSaveBackupPath_data()
+{
+ QTest::addColumn<QString>("backupFilePathPattern");
+ QTest::addColumn<QString>("expectedBackupFile");
+
+ // Absolute paths should remain absolute
+ TemporaryFile tmpFile;
+ QVERIFY(tmpFile.open());
+ tmpFile.remove();
+
+ QTest::newRow("Absolute backup path") << tmpFile.fileName() << tmpFile.fileName();
+ // relative paths should be resolved to database parent directory
+ QTest::newRow("Relative backup path (implicit)") << "other_dir/test.old.kdbx"
+ << "other_dir/test.old.kdbx";
+ QTest::newRow("Relative backup path (explicit)") << "./other_dir2/test2.old.kdbx"
+ << "other_dir2/test2.old.kdbx";
+
+ QTest::newRow("Path with placeholders") << "{DB_FILENAME}.old.kdbx"
+ << "KeePassXC.old.kdbx";
+ // empty path should be replaced with default pattern
+ QTest::newRow("Empty path") << QString("") << config()->getDefault(Config::BackupFilePathPattern).toString();
+ // {DB_FILENAME} should be replaced with database filename
+ QTest::newRow("") << "{DB_FILENAME}_.old.kdbx"
+ << "{DB_FILENAME}_.old.kdbx";
+}
+
+void TestGui::testSaveBackupPath()
+{
+ /**
+ * Tests that the backupFilePathPattern config entry is respected. We do not test patterns like {TIME} etc here
+ * as this is done in a separate test case. We do however check {DB_FILENAME} as this is a feature of the
+ * performBackup() function.
+ */
+
+ // Get test data
+ QFETCH(QString, backupFilePathPattern);
+ QFETCH(QString, expectedBackupFile);
+
+ // Enable automatic backups
+ config()->set(Config::BackupBeforeSave, true);
+ config()->set(Config::BackupFilePathPattern, backupFilePathPattern);
+
+ // Replace placeholders and resolve relative paths. This cannot be done in the _data() function as the
+ // db path/filename is not known yet
+ auto dbFileInfo = QFileInfo(m_dbFilePath);
+ if (!QDir::isAbsolutePath(expectedBackupFile)) {
+ expectedBackupFile = QDir(dbFileInfo.absolutePath()).absoluteFilePath(expectedBackupFile);
+ }
+ expectedBackupFile.replace("{DB_FILENAME}", dbFileInfo.completeBaseName());
+
+ // Save a modified database
+ auto prevName = m_db->metadata()->name();
+ m_db->metadata()->setName("testBackupPathPattern");
+
+ QTRY_COMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("testBackupPathPattern*"));
+ triggerAction("actionDatabaseSave");
+ QTRY_COMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("testBackupPathPattern"));
+
+ // Test that the backup file has the previous database name
+ checkDatabase(expectedBackupFile, prevName);
+
+ // Clean up
+ QFile(expectedBackupFile).remove();
+}
+
void TestGui::testDatabaseSettings()
{
m_db->metadata()->setName("testDatabaseSettings");
@@ -1301,7 +1367,7 @@ void TestGui::testDatabaseSettings()
QCOMPARE(m_db->kdf()->rounds(), 123456);
triggerAction("actionDatabaseSave");
- QCOMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("testDatabaseSettings"));
+ QTRY_COMPARE(m_tabWidget->tabText(m_tabWidget->currentIndex()), QString("testDatabaseSettings"));
advancedToggle->setChecked(false);
QApplication::processEvents();
@@ -1365,6 +1431,8 @@ void TestGui::testDragAndDropKdbxFiles()
const int openedDatabasesCount = m_tabWidget->count();
const QString badDatabaseFilePath(QString(KEEPASSX_TEST_DATA_DIR).append("/NotDatabase.notkdbx"));
+ const QString goodDatabaseFilePath(QString(KEEPASSX_TEST_DATA_DIR).append("/NewDatabase.kdbx"));
+
QMimeData badMimeData;
badMimeData.setUrls({QUrl::fromLocalFile(badDatabaseFilePath)});
QDragEnterEvent badDragEvent(QPoint(1, 1), Qt::LinkAction, &badMimeData, Qt::LeftButton, Qt::NoModifier);
@@ -1378,7 +1446,7 @@ void TestGui::testDragAndDropKdbxFiles()
QCOMPARE(m_tabWidget->count(), openedDatabasesCount);
QMimeData goodMimeData;
- goodMimeData.setUrls({QUrl::fromLocalFile(dbFileName)});
+ goodMimeData.setUrls({QUrl::fromLocalFile(goodDatabaseFilePath)});
QDragEnterEvent goodDragEvent(QPoint(1, 1), Qt::LinkAction, &goodMimeData, Qt::LeftButton, Qt::NoModifier);
qApp->notify(m_mainWindow.data(), &goodDragEvent);
QCOMPARE(goodDragEvent.isAccepted(), true);
@@ -1700,17 +1768,18 @@ void TestGui::addCannedEntries()
QTest::mouseClick(editEntryWidgetButtonBox->button(QDialogButtonBox::Ok), Qt::LeftButton);
}
-void TestGui::checkDatabase(QString dbFileName)
+void TestGui::checkDatabase(const QString& filePath, const QString& expectedDbName)
{
- if (dbFileName.isEmpty()) {
- dbFileName = m_dbFilePath;
- }
-
auto key = QSharedPointer<CompositeKey>::create();
key->addKey(QSharedPointer<PasswordKey>::create("a"));
auto dbSaved = QSharedPointer<Database>::create();
- QVERIFY(dbSaved->open(dbFileName, key, nullptr, false));
- QCOMPARE(dbSaved->metadata()->name(), m_db->metadata()->name());
+ QVERIFY(dbSaved->open(filePath, key, nullptr, false));
+ QCOMPARE(dbSaved->metadata()->name(), expectedDbName);
+}
+
+void TestGui::checkDatabase(const QString& filePath)
+{
+ checkDatabase(filePath.isEmpty() ? m_dbFilePath : filePath, m_db->metadata()->name());
}
void TestGui::triggerAction(const QString& name)
diff --git a/tests/gui/TestGui.h b/tests/gui/TestGui.h
index 0185c544d..f9dcf8867 100644
--- a/tests/gui/TestGui.h
+++ b/tests/gui/TestGui.h
@@ -57,6 +57,8 @@ private slots:
void testSaveAs();
void testSaveBackup();
void testSave();
+ void testSaveBackupPath();
+ void testSaveBackupPath_data();
void testDatabaseSettings();
void testKeePass1Import();
void testDatabaseLocking();
@@ -67,7 +69,8 @@ private slots:
private:
void addCannedEntries();
- void checkDatabase(QString dbFileName = "");
+ void checkDatabase(const QString& filePath, const QString& expectedDbName);
+ void checkDatabase(const QString& filePath = {});
void triggerAction(const QString& name);
void dragAndDropGroup(const QModelIndex& sourceIndex,
const QModelIndex& targetIndex,