Fix merging browser keys

* Introduce protected custom data function to prevent loss during merge operations
This commit is contained in:
varjolintu 2020-05-01 11:07:14 +03:00 committed by Jonathan White
parent 48bf4fb85d
commit e367c6df95
6 changed files with 29 additions and 20 deletions

View File

@ -46,11 +46,9 @@
const QString BrowserService::KEEPASSXCBROWSER_NAME = QStringLiteral("KeePassXC-Browser Settings"); const QString BrowserService::KEEPASSXCBROWSER_NAME = QStringLiteral("KeePassXC-Browser Settings");
const QString BrowserService::KEEPASSXCBROWSER_OLD_NAME = QStringLiteral("keepassxc-browser Settings"); const QString BrowserService::KEEPASSXCBROWSER_OLD_NAME = QStringLiteral("keepassxc-browser Settings");
const QString BrowserService::ASSOCIATE_KEY_PREFIX = QStringLiteral("KPXC_BROWSER_");
static const QString KEEPASSXCBROWSER_GROUP_NAME = QStringLiteral("KeePassXC-Browser Passwords"); static const QString KEEPASSXCBROWSER_GROUP_NAME = QStringLiteral("KeePassXC-Browser Passwords");
static int KEEPASSXCBROWSER_DEFAULT_ICON = 1; static int KEEPASSXCBROWSER_DEFAULT_ICON = 1;
// These are for the settings and password conversion // These are for the settings and password conversion
const QString BrowserService::LEGACY_ASSOCIATE_KEY_PREFIX = QStringLiteral("Public Key: ");
static const QString KEEPASSHTTP_NAME = QStringLiteral("KeePassHttp Settings"); static const QString KEEPASSHTTP_NAME = QStringLiteral("KeePassHttp Settings");
static const QString KEEPASSHTTP_GROUP_NAME = QStringLiteral("KeePassHttp Passwords"); static const QString KEEPASSHTTP_GROUP_NAME = QStringLiteral("KeePassHttp Passwords");
// Extra entry related options saved in custom data // Extra entry related options saved in custom data
@ -318,7 +316,7 @@ QString BrowserService::storeKey(const QString& key)
return {}; return {};
} }
contains = db->metadata()->customData()->contains(ASSOCIATE_KEY_PREFIX + id); contains = db->metadata()->customData()->contains(CustomData::BrowserKeyPrefix + id);
if (contains) { if (contains) {
dialogResult = MessageBox::warning(nullptr, dialogResult = MessageBox::warning(nullptr,
tr("KeePassXC: Overwrite existing key?"), tr("KeePassXC: Overwrite existing key?"),
@ -331,7 +329,7 @@ QString BrowserService::storeKey(const QString& key)
} while (contains && dialogResult == MessageBox::Cancel); } while (contains && dialogResult == MessageBox::Cancel);
hideWindow(); hideWindow();
db->metadata()->customData()->set(ASSOCIATE_KEY_PREFIX + id, key); db->metadata()->customData()->set(CustomData::BrowserKeyPrefix + id, key);
db->metadata()->customData()->set(QString("%1_%2").arg(CustomData::Created, id), db->metadata()->customData()->set(QString("%1_%2").arg(CustomData::Created, id),
Clock::currentDateTime().toString(Qt::SystemLocaleShortDate)); Clock::currentDateTime().toString(Qt::SystemLocaleShortDate));
return id; return id;
@ -344,7 +342,7 @@ QString BrowserService::getKey(const QString& id)
return {}; return {};
} }
return db->metadata()->customData()->value(ASSOCIATE_KEY_PREFIX + id); return db->metadata()->customData()->value(CustomData::BrowserKeyPrefix + id);
} }
QJsonArray BrowserService::findMatchingEntries(const QString& dbid, QJsonArray BrowserService::findMatchingEntries(const QString& dbid,
@ -590,7 +588,7 @@ QList<Entry*> BrowserService::searchEntries(const QString& url, const QString& s
// Check if database is connected with KeePassXC-Browser // Check if database is connected with KeePassXC-Browser
auto databaseConnected = [&](const QSharedPointer<Database>& db) { auto databaseConnected = [&](const QSharedPointer<Database>& db) {
for (const StringPair& keyPair : keyList) { for (const StringPair& keyPair : keyList) {
QString key = db->metadata()->customData()->value(ASSOCIATE_KEY_PREFIX + keyPair.first); QString key = db->metadata()->customData()->value(CustomData::BrowserKeyPrefix + keyPair.first);
if (!key.isEmpty() && keyPair.second == key) { if (!key.isEmpty() && keyPair.second == key) {
return true; return true;
} }
@ -1121,13 +1119,14 @@ int BrowserService::moveKeysToCustomData(Entry* entry, QSharedPointer<Database>
{ {
int keyCounter = 0; int keyCounter = 0;
for (const auto& key : entry->attributes()->keys()) { for (const auto& key : entry->attributes()->keys()) {
if (key.contains(LEGACY_ASSOCIATE_KEY_PREFIX)) { if (key.contains(CustomData::BrowserLegacyKeyPrefix)) {
QString publicKey = key; QString publicKey = key;
publicKey.remove(LEGACY_ASSOCIATE_KEY_PREFIX); publicKey.remove(CustomData::BrowserLegacyKeyPrefix);
// Add key to database custom data // Add key to database custom data
if (db && !db->metadata()->customData()->contains(ASSOCIATE_KEY_PREFIX + publicKey)) { if (db && !db->metadata()->customData()->contains(CustomData::BrowserKeyPrefix + publicKey)) {
db->metadata()->customData()->set(ASSOCIATE_KEY_PREFIX + publicKey, entry->attributes()->value(key)); db->metadata()->customData()->set(CustomData::BrowserKeyPrefix + publicKey,
entry->attributes()->value(key));
++keyCounter; ++keyCounter;
} }
} }

View File

@ -86,8 +86,6 @@ public:
static const QString KEEPASSXCBROWSER_NAME; static const QString KEEPASSXCBROWSER_NAME;
static const QString KEEPASSXCBROWSER_OLD_NAME; static const QString KEEPASSXCBROWSER_OLD_NAME;
static const QString ASSOCIATE_KEY_PREFIX;
static const QString LEGACY_ASSOCIATE_KEY_PREFIX;
static const QString OPTION_SKIP_AUTO_SUBMIT; static const QString OPTION_SKIP_AUTO_SUBMIT;
static const QString OPTION_HIDE_ENTRY; static const QString OPTION_HIDE_ENTRY;
static const QString OPTION_ONLY_HTTP_AUTH; static const QString OPTION_ONLY_HTTP_AUTH;

View File

@ -22,6 +22,8 @@
const QString CustomData::LastModified = QStringLiteral("_LAST_MODIFIED"); const QString CustomData::LastModified = QStringLiteral("_LAST_MODIFIED");
const QString CustomData::Created = QStringLiteral("_CREATED"); const QString CustomData::Created = QStringLiteral("_CREATED");
const QString CustomData::BrowserKeyPrefix = QStringLiteral("KPXC_BROWSER_");
const QString CustomData::BrowserLegacyKeyPrefix = QStringLiteral("Public Key: ");
CustomData::CustomData(QObject* parent) CustomData::CustomData(QObject* parent)
: QObject(parent) : QObject(parent)
@ -128,6 +130,11 @@ QDateTime CustomData::getLastModified() const
return {}; return {};
} }
bool CustomData::isProtectedCustomData(const QString& key) const
{
return key.startsWith(CustomData::BrowserKeyPrefix) || key.startsWith(CustomData::Created);
}
bool CustomData::operator==(const CustomData& other) const bool CustomData::operator==(const CustomData& other) const
{ {
return (m_data == other.m_data); return (m_data == other.m_data);

View File

@ -43,11 +43,14 @@ public:
int dataSize() const; int dataSize() const;
void copyDataFrom(const CustomData* other); void copyDataFrom(const CustomData* other);
QDateTime getLastModified() const; QDateTime getLastModified() const;
bool isProtectedCustomData(const QString& key) const;
bool operator==(const CustomData& other) const; bool operator==(const CustomData& other) const;
bool operator!=(const CustomData& other) const; bool operator!=(const CustomData& other) const;
static const QString LastModified; static const QString LastModified;
static const QString Created; static const QString Created;
static const QString BrowserKeyPrefix;
static const QString BrowserLegacyKeyPrefix;
signals: signals:
void customDataModified(); void customDataModified();

View File

@ -632,7 +632,9 @@ Merger::ChangeList Merger::mergeMetadata(const MergeContext& context)
// Check missing keys from source. Remove those from target // Check missing keys from source. Remove those from target
for (const auto& key : targetCustomDataKeys) { for (const auto& key : targetCustomDataKeys) {
if (!sourceMetadata->customData()->contains(key)) { // Do not remove protected custom data
if (!sourceMetadata->customData()->contains(key)
&& !sourceMetadata->customData()->isProtectedCustomData(key)) {
auto value = targetMetadata->customData()->value(key); auto value = targetMetadata->customData()->value(key);
targetMetadata->customData()->remove(key); targetMetadata->customData()->remove(key);
changes << tr("Removed custom data %1 [%2]").arg(key, value); changes << tr("Removed custom data %1 [%2]").arg(key, value);

View File

@ -107,7 +107,7 @@ void DatabaseSettingsWidgetBrowser::removeSelectedKey()
if (itemSelectionModel) { if (itemSelectionModel) {
for (const QModelIndex& index : itemSelectionModel->selectedRows(0)) { for (const QModelIndex& index : itemSelectionModel->selectedRows(0)) {
QString key = index.data().toString(); QString key = index.data().toString();
key.insert(0, BrowserService::ASSOCIATE_KEY_PREFIX); key.insert(0, CustomData::BrowserKeyPrefix);
customData()->remove(key); customData()->remove(key);
} }
updateModel(); updateModel();
@ -125,9 +125,9 @@ void DatabaseSettingsWidgetBrowser::updateModel()
m_customDataModel->setHorizontalHeaderLabels({tr("Key"), tr("Value"), tr("Created")}); m_customDataModel->setHorizontalHeaderLabels({tr("Key"), tr("Value"), tr("Created")});
for (const QString& key : customData()->keys()) { for (const QString& key : customData()->keys()) {
if (key.startsWith(BrowserService::ASSOCIATE_KEY_PREFIX)) { if (key.startsWith(CustomData::BrowserKeyPrefix)) {
QString strippedKey = key; QString strippedKey = key;
strippedKey.remove(BrowserService::ASSOCIATE_KEY_PREFIX); strippedKey.remove(CustomData::BrowserKeyPrefix);
auto created = customData()->value(QString("%1_%2").arg(CustomData::Created, strippedKey)); auto created = customData()->value(QString("%1_%2").arg(CustomData::Created, strippedKey));
auto createdItem = new QStandardItem(created); auto createdItem = new QStandardItem(created);
createdItem->setEditable(false); createdItem->setEditable(false);
@ -174,7 +174,7 @@ void DatabaseSettingsWidgetBrowser::removeSharedEncryptionKeys()
QStringList keysToRemove; QStringList keysToRemove;
for (const QString& key : m_db->metadata()->customData()->keys()) { for (const QString& key : m_db->metadata()->customData()->keys()) {
if (key.startsWith(BrowserService::ASSOCIATE_KEY_PREFIX)) { if (key.startsWith(CustomData::BrowserKeyPrefix)) {
keysToRemove << key; keysToRemove << key;
} }
} }
@ -299,16 +299,16 @@ void DatabaseSettingsWidgetBrowser::editFinished(QStandardItem* item)
// The key is edited // The key is edited
if (item->column() == 0) { if (item->column() == 0) {
// Get the old key/value pair, remove it and replace it // Get the old key/value pair, remove it and replace it
m_valueInEdit.insert(0, BrowserService::ASSOCIATE_KEY_PREFIX); m_valueInEdit.insert(0, CustomData::BrowserKeyPrefix);
auto tempValue = customData()->value(m_valueInEdit); auto tempValue = customData()->value(m_valueInEdit);
newValue.insert(0, BrowserService::ASSOCIATE_KEY_PREFIX); newValue.insert(0, CustomData::BrowserKeyPrefix);
m_db->metadata()->customData()->remove(m_valueInEdit); m_db->metadata()->customData()->remove(m_valueInEdit);
m_db->metadata()->customData()->set(newValue, tempValue); m_db->metadata()->customData()->set(newValue, tempValue);
} else { } else {
// Replace just the value // Replace just the value
for (const QString& key : m_db->metadata()->customData()->keys()) { for (const QString& key : m_db->metadata()->customData()->keys()) {
if (key.startsWith(BrowserService::ASSOCIATE_KEY_PREFIX)) { if (key.startsWith(CustomData::BrowserKeyPrefix)) {
if (m_valueInEdit == m_db->metadata()->customData()->value(key)) { if (m_valueInEdit == m_db->metadata()->customData()->value(key)) {
m_db->metadata()->customData()->set(key, newValue); m_db->metadata()->customData()->set(key, newValue);
break; break;