From 718dddb01f7f372f1c91b13e2a0c322ed68c52eb Mon Sep 17 00:00:00 2001 From: Timon Reinold Date: Wed, 9 Jul 2025 16:57:48 +0200 Subject: [PATCH 1/8] Track public UUID of opened databases Add a map from public UUID to database tab to DatabaseTabWidget, to allow efficiently accessing Databases by public UUID. Similar to Database::s_uuidMap. This commit does not add any user-facing features. While independently created UUIDs usually don't collide, the public UUID could be manipulated, or simply duplicated by copying the file. This will simply ignore all databases opened while another database with the same public UUID is already open (even if that first database is later closed, until the second database is re-opened). --- src/gui/DatabaseTabWidget.cpp | 28 ++++++++++++++++++++++++++++ src/gui/DatabaseTabWidget.h | 3 +++ 2 files changed, 31 insertions(+) diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index c8639eb44..235b26974 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -225,6 +225,13 @@ void DatabaseTabWidget::addDatabaseTab(DatabaseWidget* dbWidget, bool inBackgrou { Q_ASSERT(dbWidget->database()); + // register public UUID before emiting databaseOpened() signal + auto publicUuid = dbWidget->database()->publicUuid(); + if (!m_publicUuidMap.contains(publicUuid)) { + // if two DBs have identical publicUuid, ignore the second one + m_publicUuidMap.insert(dbWidget->database()->publicUuid(), dbWidget); + } + // emit before index change emit databaseOpened(dbWidget); @@ -235,6 +242,7 @@ void DatabaseTabWidget::addDatabaseTab(DatabaseWidget* dbWidget, bool inBackgrou setCurrentIndex(index); } + connect(dbWidget, &DatabaseWidget::databaseReplaced, this, &DatabaseTabWidget::updatePublicUuid); connect(dbWidget, SIGNAL(requestOpenDatabase(QString, bool, QString, QString)), SLOT(addDatabaseTab(QString, bool, QString, QString))); @@ -385,6 +393,11 @@ bool DatabaseTabWidget::closeDatabaseTab(DatabaseWidget* dbWidget) return false; } + auto publicUuid = dbWidget->database()->publicUuid(); + if (m_publicUuidMap.value(publicUuid) == dbWidget) { + m_publicUuidMap.remove(publicUuid); + } + QString filePath = dbWidget->database()->filePath(); if (!dbWidget->close()) { return false; @@ -685,6 +698,11 @@ void DatabaseTabWidget::updateTabName(int index) emit tabNameChanged(); } +DatabaseWidget* DatabaseTabWidget::databaseWidgetFromPublicUuid(const QUuid& publicUuid) const +{ + return m_publicUuidMap.value(publicUuid, nullptr); +} + DatabaseWidget* DatabaseTabWidget::databaseWidgetFromIndex(int index) const { return qobject_cast(widget(index)); @@ -886,6 +904,16 @@ void DatabaseTabWidget::updateLastDatabases() } } +void DatabaseTabWidget::updatePublicUuid(const QSharedPointer& oldDb, const QSharedPointer& newDb) +{ + if (!oldDb.isNull()) { + auto widget = m_publicUuidMap.take(oldDb->publicUuid()); + if (widget == sender()) { + m_publicUuidMap.insert(newDb->publicUuid(), widget); + } + } +} + void DatabaseTabWidget::emitActiveDatabaseChanged() { emit activeDatabaseChanged(currentDatabaseWidget()); diff --git a/src/gui/DatabaseTabWidget.h b/src/gui/DatabaseTabWidget.h index a5074a84c..481f4fb89 100644 --- a/src/gui/DatabaseTabWidget.h +++ b/src/gui/DatabaseTabWidget.h @@ -43,6 +43,7 @@ public: QString tabName(int index); DatabaseWidget* currentDatabaseWidget(); DatabaseWidget* databaseWidgetFromIndex(int index) const; + DatabaseWidget* databaseWidgetFromPublicUuid(const QUuid& publicUuid) const; bool canSave(int index = -1) const; bool isModified(int index = -1) const; @@ -114,6 +115,7 @@ private slots: void handleDatabaseUnlockDialogFinished(bool accepted, DatabaseWidget* dbWidget); void handleExportError(const QString& reason); void updateLastDatabases(); + void updatePublicUuid(const QSharedPointer& oldDb, const QSharedPointer& newDb); private: QSharedPointer execNewDatabaseWizard(); @@ -127,6 +129,7 @@ private: QPointer m_importWizard; QTimer m_lockDelayTimer; bool m_databaseOpenInProgress; + QHash m_publicUuidMap; }; #endif // KEEPASSX_DATABASETABWIDGET_H From 87e2e5d6bd5ca64f8bcce43533cadd633893abf7 Mon Sep 17 00:00:00 2001 From: Timon Reinold Date: Wed, 9 Jul 2025 16:57:48 +0200 Subject: [PATCH 2/8] Add FdoSecrets/CollectionAliasDatabaseUUIDs config Add a setting to store the Freedesktop Secret Service aliases. Setting e.g. the "default" alias indicates, to which database most applications should store new secrets. This is a QVariantMap (aka QMap) as opposed to a QMap, as only the former can be stored to QSettings. Converting a QVariantMap to QMap would require copying the entire map, calling QVariant::toUuid on every value. This commit does not yet add the user interface for configuring this setting, nor does it apply the configured aliases to the actual D-Bus service. --- src/core/Config.cpp | 1 + src/core/Config.h | 1 + src/fdosecrets/FdoSecretsSettings.cpp | 33 +++++++++++++++++++++++++++ src/fdosecrets/FdoSecretsSettings.h | 13 ++++++++++- 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/core/Config.cpp b/src/core/Config.cpp index 04b724fb1..77dbdfa48 100644 --- a/src/core/Config.cpp +++ b/src/core/Config.cpp @@ -193,6 +193,7 @@ static const QHash configStrings = { {Config::FdoSecrets_ConfirmDeleteItem, {QS("FdoSecrets/ConfirmDeleteItem"), Roaming, true}}, {Config::FdoSecrets_ConfirmAccessItem, {QS("FdoSecrets/ConfirmAccessItem"), Roaming, true}}, {Config::FdoSecrets_UnlockBeforeSearch, {QS("FdoSecrets/UnlockBeforeSearch"), Roaming, true}}, + {Config::FdoSecrets_CollectionAliasDatabaseUUIDs, {QS("FdoSecrets/CollectionAliasDatabaseUUIDs"), Roaming, QVariantMap()}}, // KeeShare {Config::KeeShare_QuietSuccess, {QS("KeeShare/QuietSuccess"), Roaming, false}}, diff --git a/src/core/Config.h b/src/core/Config.h index 9385f9ef1..c36255728 100644 --- a/src/core/Config.h +++ b/src/core/Config.h @@ -172,6 +172,7 @@ public: FdoSecrets_ConfirmDeleteItem, FdoSecrets_ConfirmAccessItem, FdoSecrets_UnlockBeforeSearch, + FdoSecrets_CollectionAliasDatabaseUUIDs, KeeShare_QuietSuccess, KeeShare_Own, diff --git a/src/fdosecrets/FdoSecretsSettings.cpp b/src/fdosecrets/FdoSecretsSettings.cpp index d24bc69a1..ff31c63af 100644 --- a/src/fdosecrets/FdoSecretsSettings.cpp +++ b/src/fdosecrets/FdoSecretsSettings.cpp @@ -104,4 +104,37 @@ namespace FdoSecrets db->metadata()->customData()->set(CustomData::FdoSecretsExposedGroup, group.toString()); } + QVariantMap FdoSecretsSettings::collectionAliases() const + { + return config()->get(Config::FdoSecrets_CollectionAliasDatabaseUUIDs).toMap(); + } + + void FdoSecretsSettings::setCollectionAliases(const QVariantMap& aliases) + { + config()->set(Config::FdoSecrets_CollectionAliasDatabaseUUIDs, aliases); + emit collectionAliasesChanged(); + } + + void FdoSecretsSettings::setCollectionAlias(QString alias, QUuid publicUuid) + { + auto aliases = collectionAliases(); + auto it = std::as_const(aliases).lowerBound(alias); + if (it != aliases.cend() && *it == publicUuid) + return; + aliases.insert(std::move(it), std::move(alias), std::move(publicUuid)); + // always signals collectionAliasesChanged(), so return above if nothing changed + setCollectionAliases(std::move(aliases)); + } + + void FdoSecretsSettings::removeCollectionAlias(const QString& alias, const QUuid& publicUuid) + { + auto aliases = collectionAliases(); + auto it = aliases.find(alias); + if (it == aliases.end() || it->toUuid() != publicUuid) + return; + aliases.erase(std::move(it)); + // always signals collectionAliasesChanged(), so return above if nothing changed + setCollectionAliases(std::move(aliases)); + } + } // namespace FdoSecrets diff --git a/src/fdosecrets/FdoSecretsSettings.h b/src/fdosecrets/FdoSecretsSettings.h index 31ab005f6..68a7f07d2 100644 --- a/src/fdosecrets/FdoSecretsSettings.h +++ b/src/fdosecrets/FdoSecretsSettings.h @@ -20,14 +20,17 @@ #include #include +#include class Database; namespace FdoSecrets { - class FdoSecretsSettings + class FdoSecretsSettings : public QObject { + Q_OBJECT + public: FdoSecretsSettings() = default; static FdoSecretsSettings* instance(); @@ -47,6 +50,11 @@ namespace FdoSecrets bool unlockBeforeSearch() const; void setUnlockBeforeSearch(bool unlockBeforeSearch); + QVariantMap collectionAliases() const; + void setCollectionAliases(const QVariantMap& aliases); + void setCollectionAlias(QString alias, QUuid publicUuuid); + void removeCollectionAlias(const QString& alias, const QUuid& publicUuid); + // Per db settings QUuid exposedGroup(const QSharedPointer& db) const; @@ -54,6 +62,9 @@ namespace FdoSecrets QUuid exposedGroup(Database* db) const; void setExposedGroup(Database* db, const QUuid& group); + signals: + void collectionAliasesChanged() const; + private: static FdoSecretsSettings* m_instance; }; From 3b9f126d7796677c6269c7931d42d33b5726b6b7 Mon Sep 17 00:00:00 2001 From: Timon Reinold Date: Wed, 9 Jul 2025 16:57:48 +0200 Subject: [PATCH 3/8] Apply configured FdoSecrets aliases to collections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create and remove Freedesktop Secret Service aliases as configured in FdoSecretSettings, and store aliases created/removed via D-Bus to FdoSecretSettings. If the "default" alias has not been configured, this works exactly like it did before. If the "default" alias has been configured, but the corresponding database isn't currently opened (not even opened-but-locked), no "default" alias is exposed. This prevents secrets from accidentally being written to another database (imagine a database being routinely shared with other people, which now suddenly contains an access token for your private email, just because the file system containing your private passwords wasn't mounted). Providing no "default" alias is already what we're doing if no database tab is opened. The implementation would have preferred a "database → Set" mapping (compare `Service::applyCollectionAliasSettings`), but "alias → database" is the natural model making invalid states (multiple databases owning the same alias) unrepresentable, requiring less validation code. --- src/fdosecrets/objects/Service.cpp | 60 +++++++++++++++++++++++++++++- src/fdosecrets/objects/Service.h | 3 ++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/fdosecrets/objects/Service.cpp b/src/fdosecrets/objects/Service.cpp index e3fcefeb5..637ca35e7 100644 --- a/src/fdosecrets/objects/Service.cpp +++ b/src/fdosecrets/objects/Service.cpp @@ -78,6 +78,12 @@ namespace FdoSecrets onDatabaseTabOpened(dbWidget, true); }); + // when a new database is opened, apply it's aliases + connect(m_databases.data(), &DatabaseTabWidget::databaseOpened, this, &Service::applyCollectionAliasSettings); + // apply aliases from settings, when they change + connect( + settings(), &FdoSecretsSettings::collectionAliasesChanged, this, &Service::applyCollectionAliasSettings); + // make default alias track current activated database connect(m_databases.data(), &DatabaseTabWidget::activeDatabaseChanged, this, &Service::ensureDefaultAlias); @@ -163,7 +169,7 @@ namespace FdoSecrets void Service::ensureDefaultAlias() { - if (m_insideEnsureDefaultAlias) { + if (m_insideEnsureDefaultAlias || m_explicitlyDefinedDefaultAlias) { return; } @@ -463,6 +469,44 @@ namespace FdoSecrets return collection->addAlias(name); } + void Service::applyCollectionAliasSettings() + { + auto aliases = settings()->collectionAliases(); + // add missing aliases + m_explicitlyDefinedDefaultAlias = false; // is there still an explicit default alias? + for (auto it = aliases.cbegin(); it != aliases.cend(); ++it) { + auto& alias = it.key(); + if (alias == DEFAULT_ALIAS) { + m_explicitlyDefinedDefaultAlias = true; // will disable this->ensureDefaultAlias() + } + + auto database = m_databases->databaseWidgetFromPublicUuid(it->toUuid()); + if (!database) + continue; // cannot add alias to database not currently opened + auto collection = m_dbToCollection.value(database, nullptr); + if (!collection) + continue; + + collection->addAlias(alias).okOrDie(); // is idempotent + } + // remove unexpected aliases + QList> aliasesToRemove = {}; + for (auto it = m_aliases.cbegin(); it != m_aliases.cend(); ++it) { + auto& alias = it.key(); + Collection* const isCollection = *it; // the collection this alias currently points to + auto shouldUuid = aliases.value(alias, {}).toUuid(); // the UUID this alias should point to + if (shouldUuid.isNull() || isCollection->backend()->database()->publicUuid() != shouldUuid) { + // don't call isCollection->removeAlias, to avoid invalidating the iterator it + aliasesToRemove.append(std::make_pair(alias, isCollection)); + } + } + for (auto toRemove : aliasesToRemove) { + toRemove.second->removeAlias(toRemove.first).okOrDie(); + } + // select default alias, if not explicitly defined + ensureDefaultAlias(); + } + Collection* Service::findCollection(const QString& alias) const { if (alias.isEmpty()) { @@ -495,11 +539,23 @@ namespace FdoSecrets { auto coll = qobject_cast(sender()); m_aliases[alias] = coll; + if (!(m_insideEnsureDefaultAlias && alias == DEFAULT_ALIAS)) { + // store dynamically created aliases to settings + settings()->setCollectionAlias(alias, coll->backend()->database()->publicUuid()); + } } void Service::onCollectionAliasRemoved(const QString& alias) { - m_aliases.remove(alias); + auto coll = m_aliases.take(alias); + if (coll + && !(m_insideEnsureDefaultAlias && alias == DEFAULT_ALIAS) + // don't persistently remove alias after collectionAboutToDelete() was emitted + && m_dbToCollection.contains(coll->backend())) { + // pass publicUuid, to avoid removing alias x to database A, when + // applyCollectionAliasSettings() removes x->B while applying this very x->A + settings()->removeCollectionAlias(alias, coll->backend()->database()->publicUuid()); + } ensureDefaultAlias(); } diff --git a/src/fdosecrets/objects/Service.h b/src/fdosecrets/objects/Service.h index 5ec7499b1..63e39db4c 100644 --- a/src/fdosecrets/objects/Service.h +++ b/src/fdosecrets/objects/Service.h @@ -143,6 +143,8 @@ namespace FdoSecrets void onCollectionAliasRemoved(const QString& alias); + void applyCollectionAliasSettings(); + private: bool initialize(); @@ -173,6 +175,7 @@ namespace FdoSecrets QList m_sessions{}; bool m_insideEnsureDefaultAlias{false}; + bool m_explicitlyDefinedDefaultAlias{false}; bool m_unlockingAnyDatabase{false}; // list of db currently has unlock dialog shown QHash m_unlockingDb{}; From f2361eb3851107ec743b1048333344c119a3a155 Mon Sep 17 00:00:00 2001 From: Timon Reinold Date: Wed, 9 Jul 2025 16:57:48 +0200 Subject: [PATCH 4/8] Add GUI to configure FdoSecrets aliases Fixes #8479 The QComboBox'es created by DatabaseUuidDelegate::createEditor are not updated when database tabs are created or destroyed. A new editor seems to be created each time the user starts editing a field, so this should not be a problem. --- share/translations/keepassxc_en.ts | 36 +++ src/fdosecrets/widgets/SettingsModels.cpp | 265 ++++++++++++++++++ src/fdosecrets/widgets/SettingsModels.h | 43 +++ .../widgets/SettingsWidgetFdoSecrets.cpp | 87 ++++++ .../widgets/SettingsWidgetFdoSecrets.h | 6 + .../widgets/SettingsWidgetFdoSecrets.ui | 72 +++++ 6 files changed, 509 insertions(+) diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 3b34abb9f..40b236c99 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -4557,6 +4557,25 @@ This will leave your passwords and sensitive information vulnerable! + + FdoSecrets::SettingsAliasesModel + + No database selected. + + + + The database with this UUID is not currently opened. + + + + Collection Alias + + + + Database + + + FdoSecrets::SettingsClientModel @@ -10069,6 +10088,23 @@ This option is deprecated, use --set-key-file instead. <html><head/><body><p>This improves compatibility with certain applications which search for password without unlocking the database first.</p><p>But enabling this may also crash the client if the database can not be unlocked within a certain timeout. (Usually 25s, but may be a different value set in applications.) </p></body></html> + + Collection Aliases + + + + Add + + + + Remove + + + + Expose a database's exposed group under a specific alias. If the “default”-alias has not been configured here, it will point to the currently active database tab. +A database needs to be unlocked and have an exposed group configured to be selectable here. + + SettingsWidgetKeeShare diff --git a/src/fdosecrets/widgets/SettingsModels.cpp b/src/fdosecrets/widgets/SettingsModels.cpp index d68ee4b0c..02f2b401d 100644 --- a/src/fdosecrets/widgets/SettingsModels.cpp +++ b/src/fdosecrets/widgets/SettingsModels.cpp @@ -244,6 +244,271 @@ namespace FdoSecrets } } + // static constexpr still requires definition before c++17 + constexpr const char* SettingsAliasesModel::ColumnNames[]; + + SettingsAliasesModel::SettingsAliasesModel(const DatabaseTabWidget* dbTabs, QObject* parent) + : QAbstractTableModel(parent) + , m_databases(dbTabs) + { + } + + void SettingsAliasesModel::setAliases(QVariantMap aliases) + { + beginResetModel(); + m_aliases = aliases; + endResetModel(); + } + + const QVariantMap& SettingsAliasesModel::aliases() const + { + return m_aliases; + } + + int SettingsAliasesModel::rowCount(const QModelIndex& parent) const + { + if (parent.isValid()) { + return 0; + } + return newRowIndex() + extraNewRow; + } + + int SettingsAliasesModel::newRowIndex() const + { + return m_aliases.size(); + } + + int SettingsAliasesModel::columnCount(const QModelIndex& parent) const + { + if (parent.isValid()) { + return 0; + } + return sizeof(ColumnNames) / sizeof(ColumnNames[0]); + } + + QVariant SettingsAliasesModel::headerData(int section, Qt::Orientation orientation, int role) const + { + if (orientation != Qt::Horizontal) { + return {}; + } + + if (role != Qt::DisplayRole) { + return {}; + } + + if (section < 0 || section >= columnCount({})) { + return {}; + } + + return qApp->translate(metaObject()->className(), ColumnNames[section]); + } + + QVariantMap::const_iterator SettingsAliasesModel::rowAlias(const QModelIndex& index) const + { + return m_aliases.cbegin() + index.row(); + } + + QVariantMap::iterator SettingsAliasesModel::rowAlias(const QModelIndex& index) + { + return m_aliases.begin() + index.row(); + } + + QVariant SettingsAliasesModel::data(const QModelIndex& index, int role) const + { + if (!index.isValid()) { + return {}; + } + if (index.model() != this) { + return {}; + } + if (index.row() >= rowCount({}) || index.column() >= columnCount({})) { + return {}; + } + if (index.row() == newRowIndex()) { // final empty row, to allow adding more aliases + return {}; + } + + switch (index.column()) { + case ColumnAlias: + return dataForCollectionAlias(rowAlias(index).key(), role); + case ColumnDatabase: + return dataForDatabase(rowAlias(index)->toUuid(), role); + default: + return {}; + } + } + + QVariant SettingsAliasesModel::dataForCollectionAlias(const QString& alias, int role) const + { + switch (role) { + case Qt::DisplayRole: + case Qt::EditRole: { + return alias; + } + default: + return {}; + } + } + + QVariant SettingsAliasesModel::dataForDatabase(const QUuid& publicUuid, int role) const + { + switch (role) { + case Qt::EditRole: { + return publicUuid; // initial value for editor for this cell + } + } + + auto dbWidget = m_databases->databaseWidgetFromPublicUuid(publicUuid); + if (dbWidget) { + auto db = dbWidget->database(); + switch (role) { + case Qt::DisplayRole: { + return dbWidget->displayName(); + } + case Qt::ToolTipRole: + return db->filePath(); + default: + return {}; + } + } else if (publicUuid.isNull()) { + switch (role) { + case Qt::DisplayRole: { + return tr("No database selected."); + } + case Qt::FontRole: { + QFont font; + font.setItalic(true); + return font; + } + default: + return {}; + } + } else { + switch (role) { + case Qt::DisplayRole: { + return publicUuid; + } + case Qt::ToolTipRole: + return tr("The database with this UUID is not currently opened."); + case Qt::FontRole: { + QFont font; + font.setItalic(true); + return font; + } + default: + return {}; + } + } + } + + void SettingsAliasesModel::moveAlias(const QModelIndex& index, + const QString& prevAlias, + QString nextAlias, + QVariant database) + { + const auto destIt = std::as_const(m_aliases).lowerBound(nextAlias); + // beginMoveRows takes that element's current index, in front of which we insert + auto destRowIdx = std::distance(m_aliases.cbegin(), destIt); + // inserting directly above or below the element to be removed doesn't move + const bool willMove = destRowIdx != index.row() && destRowIdx - 1 != index.row(); + if (willMove) { + // move index.row() to destRowIdx + beginMoveRows({}, index.row(), index.row(), {}, destRowIdx); + } + const bool newRow = index.row() == newRowIndex(); + m_aliases.insert(std::move(destIt), std::move(nextAlias), std::move(database)); + if (newRow) { + // remove the special "newRow", which does not correspond to m_aliases + extraNewRow = false; // cannot nest beginMoveRows and beginInsertRows + } else { + // remove a normal row, which does correspond to an m_aliases entry + m_aliases.remove(prevAlias); + } + if (willMove) { + // one insert and one remove -> we moved a row + endMoveRows(); + } + // during this move, we might have also changed the cell values + if (index.row() < destRowIdx) { + --destRowIdx; // shifted up, when we removed index.row() + } + emit dataChanged(this->index(destRowIdx, ColumnAlias), this->index(destRowIdx, ColumnDatabase)); + if (newRow) { + // create a new "newRow", if we (re)moved the old one + beginInsertRows({}, newRowIndex(), newRowIndex()); + extraNewRow = true; // increases this->rowCount() by one + endInsertRows(); + } + } + + bool SettingsAliasesModel::setData(const QModelIndex& index, const QVariant& value, int role) + { + switch (index.column()) { + case ColumnAlias: { + QString prevAlias = ""; + QString nextAlias = value.toString(); + QVariant database = QUuid(); + if (index.row() == newRowIndex()) { + if (nextAlias.isEmpty()) + return false; // don't add empty alias + if (m_aliases.contains(nextAlias)) + return false; // don't override existing alias + } else { + auto prevRow = rowAlias(index); + prevAlias = prevRow.key(); + database = prevRow.value(); + if (nextAlias.isEmpty()) { + beginRemoveRows({}, index.row(), index.row()); + m_aliases.erase(prevRow); + endRemoveRows(); + return true; // edit to empty -> remove row + } + if (nextAlias == prevAlias) { + return true; // editor didn't change this value + } + if (m_aliases.contains(nextAlias)) { + return false; // refuse duplicate alias + } + } + moveAlias(index, prevAlias, std::move(nextAlias), std::move(database)); + return true; + } + case ColumnDatabase: { + auto nextDatabase = value.toUuid(); + QString alias = "default"; + if (index.row() == newRowIndex()) { + // find a new unique alias name + int i = 1; + while (m_aliases.contains(alias)) { + alias = QString("alias%1").arg(i++); + } + // insert new alias for selected database + moveAlias(index, "", std::move(alias), std::move(nextDatabase)); + } else { + // just update this value (won't move/insert/remove any rows) + *rowAlias(index) = nextDatabase; + emit dataChanged(index, index); + } + return true; + } + default: + return QAbstractTableModel::setData(index, value, role); + } + } + + Qt::ItemFlags SettingsAliasesModel::flags(const QModelIndex& index) const + { + // all table cells are editable (see SettingsAliasesModel::setData()) + return QAbstractTableModel::flags(index) | Qt::ItemIsEditable; + } + + void SettingsAliasesModel::removeRow(int row) + { + beginRemoveRows({}, row, row); + m_aliases.erase(m_aliases.begin() + row); + endRemoveRows(); + } + // static constexpr still requires definition before c++17 constexpr const char* SettingsClientModel::ColumnNames[]; diff --git a/src/fdosecrets/widgets/SettingsModels.h b/src/fdosecrets/widgets/SettingsModels.h index a6cdf41de..c9de862c4 100644 --- a/src/fdosecrets/widgets/SettingsModels.h +++ b/src/fdosecrets/widgets/SettingsModels.h @@ -21,6 +21,7 @@ #include "fdosecrets/dbus/DBusClient.h" #include +#include class DatabaseTabWidget; class DatabaseWidget; @@ -72,6 +73,48 @@ namespace FdoSecrets class DBusMgr; + class SettingsAliasesModel : public QAbstractTableModel + { + Q_OBJECT + public: + explicit SettingsAliasesModel(const DatabaseTabWidget* dbTabs, QObject* parent = nullptr); + + void setAliases(QVariantMap aliases); + const QVariantMap& aliases() const; + void removeRow(int row); + + int newRowIndex() const; + int rowCount(const QModelIndex& parent) const override; + int columnCount(const QModelIndex& parent) const override; + QVariant data(const QModelIndex& index, int role) const override; + bool setData(const QModelIndex& index, const QVariant& value, int role = Qt::EditRole) override; + Qt::ItemFlags flags(const QModelIndex& index) const override; + QVariant headerData(int section, Qt::Orientation orientation, int role) const override; + + enum Column + { + ColumnAlias, + ColumnDatabase, + }; + static constexpr const char* ColumnNames[] = { + QT_TRANSLATE_NOOP("FdoSecrets::SettingsAliasesModel", "Collection Alias"), + QT_TRANSLATE_NOOP("FdoSecrets::SettingsAliasesModel", "Database"), + }; + + private: + QVariantMap::const_iterator rowAlias(const QModelIndex&) const; + QVariantMap::iterator rowAlias(const QModelIndex&); + void moveAlias(const QModelIndex& index, const QString& prevAlias, QString nextAlias, QVariant database); + QVariant dataForCollectionAlias(const QString& alias, int role) const; + QVariant dataForDatabase(const QUuid& publicUuid, int role) const; + + private: + const DatabaseTabWidget* m_databases; + QVariantMap m_aliases; + // whether to include an empty row at the bottom, for new alias entry + bool extraNewRow = true; + }; + class SettingsClientModel : public QAbstractTableModel { Q_OBJECT diff --git a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp index 692a7561b..fbd09cfb0 100644 --- a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp +++ b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp @@ -16,6 +16,7 @@ */ #include "SettingsWidgetFdoSecrets.h" +#include "gui/DatabaseTabWidget.h" #include "ui_SettingsWidgetFdoSecrets.h" #include "fdosecrets/FdoSecretsPlugin.h" @@ -27,9 +28,11 @@ #include "gui/DatabaseWidget.h" #include +#include #include using FdoSecrets::DBusClientPtr; +using FdoSecrets::SettingsAliasesModel; using FdoSecrets::SettingsClientModel; using FdoSecrets::SettingsDatabaseModel; @@ -215,6 +218,61 @@ namespace }; } // namespace +class DatabaseUuidDelegate : public QStyledItemDelegate +{ + Q_OBJECT + +public: + DatabaseUuidDelegate(DatabaseTabWidget* dbTabs, QObject* parent = nullptr) + : QStyledItemDelegate(parent) + , m_dbTabs(dbTabs) + { + } + + QWidget* createEditor(QWidget* parent, const QStyleOptionViewItem&, const QModelIndex&) const override + { + QComboBox* e = new QComboBox(parent); + // Add existing database tabs + for (auto i = 0; i < m_dbTabs->count(); ++i) { + auto dbWidget = m_dbTabs->databaseWidgetFromIndex(i); + auto db = dbWidget->database(); + if (!FdoSecrets::settings()->exposedGroup(db).isNull()) { + e->insertItem(i, dbWidget->displayName(), db->publicUuid()); + e->setItemData(i, db->filePath(), Qt::ToolTipRole); + } + } + return e; + } + + void setEditorData(QWidget* editor, const QModelIndex& index) const override + { + const QVariant current = index.model()->data(index, Qt::EditRole); + if (current.toUuid().isNull()) + return; // no previously selected db + + auto e = widget(editor); + auto idx = e->findData(current); + if (idx < 0) { // UUID of not currently opened db in model, add to editor + idx = e->count(); + e->insertItem(idx, current.toUuid().toString(), current); + } + e->setCurrentIndex(idx); + } + + void setModelData(QWidget* editor, QAbstractItemModel* model, const QModelIndex& index) const override + { + model->setData(index, widget(editor)->currentData()); + } + +private: + static QComboBox* widget(QWidget* editor) + { + return static_cast(editor); + } + + const DatabaseTabWidget* const m_dbTabs; +}; + SettingsWidgetFdoSecrets::SettingsWidgetFdoSecrets(FdoSecretsPlugin* plugin, QWidget* parent) : QWidget(parent) , m_ui(new Ui::SettingsWidgetFdoSecrets()) @@ -224,6 +282,33 @@ SettingsWidgetFdoSecrets::SettingsWidgetFdoSecrets(FdoSecretsPlugin* plugin, QWi m_ui->warningMsg->setHidden(true); m_ui->warningMsg->setCloseButtonVisible(false); + m_aliasesModel = new SettingsAliasesModel(plugin->dbTabs(), this); + m_ui->tableAliases->setModel(m_aliasesModel); + + auto databaseDelegate = new DatabaseUuidDelegate(plugin->dbTabs(), this); + m_ui->tableAliases->setItemDelegateForColumn(SettingsAliasesModel::ColumnDatabase, databaseDelegate); + + connect(m_ui->addAliasButton, &QPushButton::clicked, [&]() { + auto index = + m_aliasesModel->index(m_aliasesModel->newRowIndex(), FdoSecrets::SettingsAliasesModel::ColumnAlias); + m_ui->tableAliases->setCurrentIndex(index); + m_ui->tableAliases->edit(index); + }); + connect(m_ui->removeAliasButton, &QPushButton::clicked, [&]() { + m_aliasesModel->removeRow(m_ui->tableAliases->currentIndex().row()); + }); + connect(m_ui->tableAliases->selectionModel(), + &QItemSelectionModel::currentChanged, + [&](const QModelIndex& current, const QModelIndex&) { + m_ui->removeAliasButton->setEnabled(current.isValid()); + }); + + auto aliasesViewHeader = m_ui->tableAliases->horizontalHeader(); + aliasesViewHeader->setSelectionMode(QAbstractItemView::NoSelection); + aliasesViewHeader->setSectionsClickable(false); + aliasesViewHeader->setSectionResizeMode(QHeaderView::ResizeToContents); + aliasesViewHeader->setSectionResizeMode(SettingsAliasesModel::ColumnDatabase, QHeaderView::Stretch); + auto clientModel = new SettingsClientModel(*plugin->dbus(), this); m_ui->tableClients->setModel(clientModel); installWidgetItemDelegate(m_ui->tableClients, @@ -273,6 +358,7 @@ void SettingsWidgetFdoSecrets::loadSettings() m_ui->confirmDeleteItem->setChecked(FdoSecrets::settings()->confirmDeleteItem()); m_ui->confirmAccessItem->setChecked(FdoSecrets::settings()->confirmAccessItem()); m_ui->unlockBeforeSearch->setChecked(FdoSecrets::settings()->unlockBeforeSearch()); + m_aliasesModel->setAliases(FdoSecrets::settings()->collectionAliases()); } void SettingsWidgetFdoSecrets::saveSettings() @@ -282,6 +368,7 @@ void SettingsWidgetFdoSecrets::saveSettings() FdoSecrets::settings()->setConfirmDeleteItem(m_ui->confirmDeleteItem->isChecked()); FdoSecrets::settings()->setConfirmAccessItem(m_ui->confirmAccessItem->isChecked()); FdoSecrets::settings()->setUnlockBeforeSearch(m_ui->unlockBeforeSearch->isChecked()); + FdoSecrets::settings()->setCollectionAliases(m_aliasesModel->aliases()); } void SettingsWidgetFdoSecrets::showEvent(QShowEvent* event) diff --git a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.h b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.h index df7bcf7ff..6ab38bea0 100644 --- a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.h +++ b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.h @@ -20,11 +20,16 @@ #include "gui/MessageWidget.h" +#include #include class QAbstractItemView; class FdoSecretsPlugin; +namespace FdoSecrets +{ + class SettingsAliasesModel; +} namespace Ui { @@ -52,6 +57,7 @@ protected: private: QScopedPointer m_ui; FdoSecretsPlugin* m_plugin; + QPointer m_aliasesModel; QTimer m_checkTimer; }; diff --git a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.ui b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.ui index 9c7ddb007..4a79abf4c 100644 --- a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.ui +++ b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.ui @@ -139,6 +139,78 @@ + + + Collection Aliases + + + + + + Expose a database's exposed group under a specific alias. If the “default”-alias has not been configured here, it will point to the currently active database tab. +A database needs to be unlocked and have an exposed group configured to be selectable here. + + + true + + + + + + + + + Qt::StrongFocus + + + QAbstractItemView::EditKeyPressed|QAbstractItemView::SelectedClicked + + + QAbstractItemView::SingleSelection + + + QAbstractItemView::SelectRows + + + false + + + + + + + + + Add + + + + + + + false + + + Remove + + + + + + + Qt::Vertical + + + QSizePolicy::Expanding + + + + + + + + + Authorization From 913c912677aa01949b39d6eb486fa69e7337d4e6 Mon Sep 17 00:00:00 2001 From: Timon Reinold Date: Wed, 9 Jul 2025 16:57:48 +0200 Subject: [PATCH 5/8] Remove FdoSecretsSettings::collectionAliasesChanged() Remove the collectionAliasesChanged() signal from FdoSecretsSettings to avoid making it a QObject. Instead connect to Config::changed and check which Config key changed. This does break the abstraction layer that FdoSecretsSettings was between Service and Config, and requires comparing the key each time any config value is updated, but averts the QObject overhead for FdoSecretsSettings. Suggested-by: Jonathan White (https://github.com/keepassxreboot/keepassxc/pull/12252#discussion_r2181100088) --- src/fdosecrets/FdoSecretsSettings.cpp | 1 - src/fdosecrets/FdoSecretsSettings.h | 7 +------ src/fdosecrets/objects/Service.cpp | 10 ++++++++-- src/fdosecrets/objects/Service.h | 2 ++ 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/fdosecrets/FdoSecretsSettings.cpp b/src/fdosecrets/FdoSecretsSettings.cpp index ff31c63af..30376c84b 100644 --- a/src/fdosecrets/FdoSecretsSettings.cpp +++ b/src/fdosecrets/FdoSecretsSettings.cpp @@ -112,7 +112,6 @@ namespace FdoSecrets void FdoSecretsSettings::setCollectionAliases(const QVariantMap& aliases) { config()->set(Config::FdoSecrets_CollectionAliasDatabaseUUIDs, aliases); - emit collectionAliasesChanged(); } void FdoSecretsSettings::setCollectionAlias(QString alias, QUuid publicUuid) diff --git a/src/fdosecrets/FdoSecretsSettings.h b/src/fdosecrets/FdoSecretsSettings.h index 68a7f07d2..ea0b94437 100644 --- a/src/fdosecrets/FdoSecretsSettings.h +++ b/src/fdosecrets/FdoSecretsSettings.h @@ -27,10 +27,8 @@ class Database; namespace FdoSecrets { - class FdoSecretsSettings : public QObject + class FdoSecretsSettings { - Q_OBJECT - public: FdoSecretsSettings() = default; static FdoSecretsSettings* instance(); @@ -62,9 +60,6 @@ namespace FdoSecrets QUuid exposedGroup(Database* db) const; void setExposedGroup(Database* db, const QUuid& group); - signals: - void collectionAliasesChanged() const; - private: static FdoSecretsSettings* m_instance; }; diff --git a/src/fdosecrets/objects/Service.cpp b/src/fdosecrets/objects/Service.cpp index 637ca35e7..5641f041a 100644 --- a/src/fdosecrets/objects/Service.cpp +++ b/src/fdosecrets/objects/Service.cpp @@ -81,8 +81,7 @@ namespace FdoSecrets // when a new database is opened, apply it's aliases connect(m_databases.data(), &DatabaseTabWidget::databaseOpened, this, &Service::applyCollectionAliasSettings); // apply aliases from settings, when they change - connect( - settings(), &FdoSecretsSettings::collectionAliasesChanged, this, &Service::applyCollectionAliasSettings); + connect(config(), &Config::changed, this, &Service::handleSettingsChanged); // make default alias track current activated database connect(m_databases.data(), &DatabaseTabWidget::activeDatabaseChanged, this, &Service::ensureDefaultAlias); @@ -469,6 +468,13 @@ namespace FdoSecrets return collection->addAlias(name); } + void Service::handleSettingsChanged(Config::ConfigKey key) + { + if (key == Config::FdoSecrets_CollectionAliasDatabaseUUIDs) { + applyCollectionAliasSettings(); + } + } + void Service::applyCollectionAliasSettings() { auto aliases = settings()->collectionAliases(); diff --git a/src/fdosecrets/objects/Service.h b/src/fdosecrets/objects/Service.h index 63e39db4c..24068b854 100644 --- a/src/fdosecrets/objects/Service.h +++ b/src/fdosecrets/objects/Service.h @@ -18,6 +18,7 @@ #ifndef KEEPASSXC_FDOSECRETS_SERVICE_H #define KEEPASSXC_FDOSECRETS_SERVICE_H +#include "core/Config.h" #include "fdosecrets/dbus/DBusClient.h" #include "fdosecrets/dbus/DBusObject.h" @@ -143,6 +144,7 @@ namespace FdoSecrets void onCollectionAliasRemoved(const QString& alias); + void handleSettingsChanged(Config::ConfigKey key); void applyCollectionAliasSettings(); private: From 86677600990a9d9355fb6e2d460973699b990262 Mon Sep 17 00:00:00 2001 From: Timon Reinold Date: Wed, 9 Jul 2025 16:57:48 +0200 Subject: [PATCH 6/8] Avoid deprecated operator+(QMap::iterator, int) This operator+ is deprecated since Qt6.2: https://doc.qt.io/archives/qt-6.2/qmap-iterator.html. QMap is implemented as a red/black tree, presumably without storing subtree sizes, so this is a linear lookup. We need to call this for each displayed alias (multiple times), so linear complexity for lookup by rank is a bummer. But presumably nobody's going to have that many aliases, that this actually matters. Suggested-by: Copilot --- src/fdosecrets/widgets/SettingsModels.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/fdosecrets/widgets/SettingsModels.cpp b/src/fdosecrets/widgets/SettingsModels.cpp index 02f2b401d..98f218477 100644 --- a/src/fdosecrets/widgets/SettingsModels.cpp +++ b/src/fdosecrets/widgets/SettingsModels.cpp @@ -25,6 +25,7 @@ #include "gui/Icons.h" #include +#include namespace FdoSecrets { @@ -305,12 +306,12 @@ namespace FdoSecrets QVariantMap::const_iterator SettingsAliasesModel::rowAlias(const QModelIndex& index) const { - return m_aliases.cbegin() + index.row(); + return std::next(m_aliases.cbegin(), index.row()); } QVariantMap::iterator SettingsAliasesModel::rowAlias(const QModelIndex& index) { - return m_aliases.begin() + index.row(); + return std::next(m_aliases.begin(), index.row()); } QVariant SettingsAliasesModel::data(const QModelIndex& index, int role) const @@ -505,7 +506,7 @@ namespace FdoSecrets void SettingsAliasesModel::removeRow(int row) { beginRemoveRows({}, row, row); - m_aliases.erase(m_aliases.begin() + row); + m_aliases.erase(std::next(m_aliases.begin(), row)); endRemoveRows(); } From cb7cfdd95f3589bfbf77c2e1361797201c0af3b4 Mon Sep 17 00:00:00 2001 From: Timon Reinold Date: Wed, 9 Jul 2025 16:57:48 +0200 Subject: [PATCH 7/8] Apply PR feedback Formulate a single case switch-statement as an if-statement. If we ever want to add additional cases, just turn it back into a switch-statement. Avoid calling QComboBox::setItemData with out-of-bounds indices. While QComboBox::insertItem documents that it works with larger indices, QComboBox::setItemData does not. So instead use the actual real index for both. (Note that QComboBox::addItem also just calls "insertItem(count(), ...)", so QComboBox::addItem wouldn't be any more efficient, but this way we see both calls actually using the same index.) Suggested by CodeQL (switch -> if) and Copilot (QComboBox indices). --- src/fdosecrets/widgets/SettingsModels.cpp | 4 +--- src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp | 5 +++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/fdosecrets/widgets/SettingsModels.cpp b/src/fdosecrets/widgets/SettingsModels.cpp index 98f218477..e7b12b477 100644 --- a/src/fdosecrets/widgets/SettingsModels.cpp +++ b/src/fdosecrets/widgets/SettingsModels.cpp @@ -353,11 +353,9 @@ namespace FdoSecrets QVariant SettingsAliasesModel::dataForDatabase(const QUuid& publicUuid, int role) const { - switch (role) { - case Qt::EditRole: { + if (role == Qt::EditRole) { return publicUuid; // initial value for editor for this cell } - } auto dbWidget = m_databases->databaseWidgetFromPublicUuid(publicUuid); if (dbWidget) { diff --git a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp index fbd09cfb0..7e6a8ad33 100644 --- a/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp +++ b/src/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp @@ -237,8 +237,9 @@ public: auto dbWidget = m_dbTabs->databaseWidgetFromIndex(i); auto db = dbWidget->database(); if (!FdoSecrets::settings()->exposedGroup(db).isNull()) { - e->insertItem(i, dbWidget->displayName(), db->publicUuid()); - e->setItemData(i, db->filePath(), Qt::ToolTipRole); + auto idx = e->count(); + e->insertItem(idx, dbWidget->displayName(), db->publicUuid()); + e->setItemData(idx, db->filePath(), Qt::ToolTipRole); } } return e; From a5531a0350d5b5887427e341062b7f9b7e3127f2 Mon Sep 17 00:00:00 2001 From: Timon Reinold Date: Wed, 9 Jul 2025 16:57:48 +0200 Subject: [PATCH 8/8] Test configured FdoSecrets aliases Verify that configured Freedesktop Secret Service aliases are exposed on the DBus interface. Also run some tests against SettingsAliasesModel, using Qt's QAbstractItemModelTester, which tries to find violations of QAbstractItemModel's invariants. Including that we apparently shouldn't return flags for invalid indices, so check the index's validity in SettingsAliasesModel::flags(). --- src/fdosecrets/widgets/SettingsModels.cpp | 4 + tests/gui/TestGuiFdoSecrets.cpp | 194 ++++++++++++++++++++++ tests/gui/TestGuiFdoSecrets.h | 6 + 3 files changed, 204 insertions(+) diff --git a/src/fdosecrets/widgets/SettingsModels.cpp b/src/fdosecrets/widgets/SettingsModels.cpp index e7b12b477..3146b7697 100644 --- a/src/fdosecrets/widgets/SettingsModels.cpp +++ b/src/fdosecrets/widgets/SettingsModels.cpp @@ -497,6 +497,10 @@ namespace FdoSecrets Qt::ItemFlags SettingsAliasesModel::flags(const QModelIndex& index) const { + if (!index.isValid() || index.model() != this || index.row() >= rowCount({}) + || index.column() >= columnCount({})) { + return {}; + } // all table cells are editable (see SettingsAliasesModel::setData()) return QAbstractTableModel::flags(index) | Qt::ItemIsEditable; } diff --git a/tests/gui/TestGuiFdoSecrets.cpp b/tests/gui/TestGuiFdoSecrets.cpp index fc7e218ea..319d26a36 100644 --- a/tests/gui/TestGuiFdoSecrets.cpp +++ b/tests/gui/TestGuiFdoSecrets.cpp @@ -29,6 +29,7 @@ #include "core/Global.h" #include "core/Tools.h" #include "crypto/Crypto.h" +#include "fdosecrets/widgets/SettingsModels.h" #include "gui/Application.h" #include "gui/DatabaseTabWidget.h" #include "gui/FileDialog.h" @@ -43,6 +44,7 @@ #include #include #include +#include #include int main(int argc, char* argv[]) @@ -202,6 +204,7 @@ void TestGuiFdoSecrets::init() VERIFY(m_dbWidget->save()); // enforce consistent default settings at the beginning + FdoSecrets::settings()->setCollectionAliases({}); FdoSecrets::settings()->setUnlockBeforeSearch(false); FdoSecrets::settings()->setShowNotification(false); FdoSecrets::settings()->setConfirmAccessItem(false); @@ -212,6 +215,7 @@ void TestGuiFdoSecrets::init() void TestGuiFdoSecrets::cleanup() { // restore to default settings + FdoSecrets::settings()->setCollectionAliases({}); FdoSecrets::settings()->setUnlockBeforeSearch(false); FdoSecrets::settings()->setShowNotification(false); FdoSecrets::settings()->setConfirmAccessItem(false); @@ -1602,6 +1606,196 @@ void TestGuiFdoSecrets::testDefaultAliasAlwaysPresent() DBUS_COMPARE(coll->locked(), false); } +void TestGuiFdoSecrets::testConfiguredAlias() +{ + + const QString newalias = "newalias"; + + auto service = enableService(); + VERIFY(service); + + DBUS_GET(prePath, service->ReadAlias(newalias)); + COMPARE(prePath, QDBusObjectPath("/")); + + FdoSecrets::settings()->setCollectionAlias(newalias, m_db->publicUuid()); + + DBUS_GET(collPath, service->ReadAlias(newalias)); + auto coll = getProxy(collPath); + VERIFY(coll); + + FdoSecrets::settings()->removeCollectionAlias(newalias, m_db->publicUuid()); + + DBUS_GET(postPath, service->ReadAlias(newalias)); + COMPARE(postPath, QDBusObjectPath("/")); +} + +void TestGuiFdoSecrets::testConfiguredDefaultAlias() +{ + const QString defaultAlias = "default"; + + auto service = enableService(); + VERIFY(service); + + // read original default alias + DBUS_GET(oldCollPath, service->ReadAlias(defaultAlias)); + // create a second collection (selects the new db) + QDBusObjectPath newCollPath; + { + QSignalSpy spyCollectionCreated(service.data(), SIGNAL(CollectionCreated(QDBusObjectPath))); + VERIFY(spyCollectionCreated.isValid()); + + DBUS_GET2(collPath, + promptPath, + service->CreateCollection({{DBUS_INTERFACE_SECRET_COLLECTION + ".Label", "Test NewDB"}}, "mydatadb")); + auto prompt = getProxy(promptPath); + DBUS_VERIFY(prompt->Prompt("")); + VERIFY(driveNewDatabaseWizard()); + + VERIFY(waitForSignal(spyCollectionCreated, 1)); + auto args = spyCollectionCreated.takeFirst(); + COMPARE(args.size(), 1); + newCollPath = args.at(0).value(); + } + VERIFY2(oldCollPath.path() != newCollPath.path(), oldCollPath.path().toStdString().data()); + COMPARE(m_tabWidget->count(), 2); + COMPARE(m_tabWidget->currentIndex(), 1); + + // read original default alias + DBUS_GET(origDefaultPath, service->ReadAlias(defaultAlias)); + COMPARE(origDefaultPath, newCollPath); + + // change default alias (back to the old db) + FdoSecrets::settings()->setCollectionAlias(defaultAlias, m_db->publicUuid()); + + // verify configuration had effect + DBUS_GET(configuredDefaultPath, service->ReadAlias(defaultAlias)); + COMPARE(configuredDefaultPath, oldCollPath); + auto configuredDefaultColl = getProxy(configuredDefaultPath); + VERIFY(configuredDefaultColl); + // verify, that the default alias no longer follows the active tab + m_tabWidget->setCurrentIndex(0); + DBUS_GET(configuredDefaultPath0, service->ReadAlias(defaultAlias)); + COMPARE(configuredDefaultPath0, oldCollPath); + m_tabWidget->setCurrentIndex(1); + DBUS_GET(configuredDefaultPath1, service->ReadAlias(defaultAlias)); + COMPARE(configuredDefaultPath1, oldCollPath); + + // remove default alias configuration + FdoSecrets::settings()->removeCollectionAlias(defaultAlias, m_db->publicUuid()); + + // verify that original default alias got restored + DBUS_GET(restoredDefaultPath, service->ReadAlias(defaultAlias)); + COMPARE(restoredDefaultPath, origDefaultPath); + // ... and changes with the active tab + m_tabWidget->setCurrentIndex(0); + DBUS_GET(restoredDefaultPath0, service->ReadAlias(defaultAlias)); + COMPARE(restoredDefaultPath0, oldCollPath); + m_tabWidget->setCurrentIndex(1); + DBUS_GET(restoredDefaultPath1, service->ReadAlias(defaultAlias)); + COMPARE(restoredDefaultPath1, newCollPath); +} + +void TestGuiFdoSecrets::testConfiguredUnavailableAlias() +{ + const QString alias = "unavailableAlias"; + auto service = enableService(); + VERIFY(service); + // configure an alias, for which the database is not currently opened + FdoSecrets::settings()->setCollectionAlias(alias, QUuid::createUuid()); + // check that this alias is not exposed + DBUS_GET(path, service->ReadAlias(alias)); + COMPARE(path, QDBusObjectPath("/")); +} + +void TestGuiFdoSecrets::testSettingsAliasesModelDisplay() +{ + FdoSecrets::SettingsAliasesModel model{m_tabWidget}; + QAbstractItemModelTester tester{&model}; + + model.setAliases({{"alias", m_db->publicUuid()}}); + QCOMPARE(model.data(model.index(0, 1), Qt::DisplayRole), m_dbWidget->displayName()); + QCOMPARE(model.data(model.index(0, 1), Qt::ToolTipRole), m_db->filePath()); + QCOMPARE(model.data(model.index(0, 1), Qt::EditRole), m_db->publicUuid()); +} + +void TestGuiFdoSecrets::testSettingsAliasesModel() +{ + FdoSecrets::SettingsAliasesModel model{m_tabWidget}; + QAbstractItemModelTester tester{&model}; + + const QVariant defaultUuid = QUuid::createUuid(); + const QVariant otherUuid = QUuid::createUuid(); + const QVariant newUuid = QUuid::createUuid(); + const QVariantMap origAliases{ + {"default", defaultUuid}, + {"otherAlias", otherUuid}, + }; + model.setAliases(origAliases); + QCOMPARE(model.aliases(), origAliases); + // last row should be empty (to be filled with new entries) + QCOMPARE(model.rowCount({}), origAliases.size() + 1); + const QModelIndex nextAliasIdx = model.index(origAliases.size(), 0); + const QModelIndex nextUuidIdx = model.index(origAliases.size(), 1); + QCOMPARE(model.data(nextAliasIdx, Qt::DisplayRole), QVariant()); + QCOMPARE(model.data(nextUuidIdx, Qt::DisplayRole), QVariant()); + // last row should be editable (to insert new entries) + QVERIFY(model.flags(nextAliasIdx) & Qt::ItemIsEditable); + QVERIFY(model.setData(nextAliasIdx, "newAlias")); + QVERIFY(model.aliases().contains("newAlias")); + // aliases sorted, so this should be newAlias + const QModelIndex newAliasIdx = model.index(1, 0); + const QModelIndex newUuidIdx = model.index(1, 1); + QCOMPARE(model.data(newAliasIdx, Qt::DisplayRole), "newAlias"); + QVERIFY(model.flags(newUuidIdx) & Qt::ItemIsEditable); + QVERIFY(model.setData(newUuidIdx, newUuid)); + QCOMPARE(model.aliases()["newAlias"], newUuid); + // entries can be renamed + QVERIFY(model.flags(newAliasIdx) & Qt::ItemIsEditable); + QCOMPARE(model.data(newAliasIdx, Qt::EditRole), "newAlias"); + QVERIFY(model.setData(newAliasIdx, "renamedAlias")); + QVERIFY(!model.aliases().contains("newAlias")); + // updates data model.aliases() + QCOMPARE(model.aliases()["default"], defaultUuid); + QCOMPARE(model.aliases()["otherAlias"], otherUuid); + QCOMPARE(model.aliases()["renamedAlias"], newUuid); + // updates sorted display + QCOMPARE(model.data(model.index(0, 0), Qt::DisplayRole), "default"); + QCOMPARE(model.data(model.index(1, 0), Qt::DisplayRole), "otherAlias"); + QCOMPARE(model.data(model.index(2, 0), Qt::DisplayRole), "renamedAlias"); + QCOMPARE(model.data(model.index(0, 1), Qt::DisplayRole), defaultUuid); + QCOMPARE(model.data(model.index(1, 1), Qt::DisplayRole), otherUuid); + QCOMPARE(model.data(model.index(2, 1), Qt::DisplayRole), newUuid); + + // automatically generate names for newly inserted databases + const QVariant unnamedUuid = QUuid::createUuid(); + QVERIFY(model.setData(model.index(3, 1), unnamedUuid)); + QCOMPARE(model.aliases()["alias1"], unnamedUuid); // "default" already exists + // updates sorted display + QCOMPARE(model.data(model.index(0, 0), Qt::DisplayRole), "alias1"); + QCOMPARE(model.data(model.index(1, 0), Qt::DisplayRole), "default"); + QCOMPARE(model.data(model.index(2, 0), Qt::DisplayRole), "otherAlias"); + QCOMPARE(model.data(model.index(3, 0), Qt::DisplayRole), "renamedAlias"); + QCOMPARE(model.data(model.index(0, 1), Qt::DisplayRole), unnamedUuid); + QCOMPARE(model.data(model.index(1, 1), Qt::DisplayRole), defaultUuid); + QCOMPARE(model.data(model.index(2, 1), Qt::DisplayRole), otherUuid); + QCOMPARE(model.data(model.index(3, 1), Qt::DisplayRole), newUuid); + + // remove rows + model.removeRow(1); + QCOMPARE(model.rowCount({}), 4); + QVERIFY(!model.aliases().contains("default")); + // other way of removing rows + QVERIFY(model.setData(model.index(1, 0), "")); + QCOMPARE(model.rowCount({}), 3); + QVERIFY(!model.aliases().contains("otherAlias")); + QCOMPARE(model.data(model.index(0, 0), Qt::DisplayRole), "alias1"); + QCOMPARE(model.data(model.index(1, 0), Qt::DisplayRole), "renamedAlias"); + QCOMPARE(model.data(model.index(2, 0), Qt::DisplayRole), {}); + QCOMPARE(model.data(model.index(0, 1), Qt::DisplayRole), unnamedUuid); + QCOMPARE(model.data(model.index(1, 1), Qt::DisplayRole), newUuid); + QCOMPARE(model.data(model.index(2, 1), Qt::DisplayRole), {}); +} + void TestGuiFdoSecrets::testExposeSubgroup() { auto subgroup = m_db->rootGroup()->findGroupByPath("/Homebanking/Subgroup"); diff --git a/tests/gui/TestGuiFdoSecrets.h b/tests/gui/TestGuiFdoSecrets.h index 1624eed49..55754842d 100644 --- a/tests/gui/TestGuiFdoSecrets.h +++ b/tests/gui/TestGuiFdoSecrets.h @@ -96,6 +96,12 @@ private slots: void testAlias(); void testDefaultAliasAlwaysPresent(); + void testConfiguredAlias(); + void testConfiguredDefaultAlias(); + void testConfiguredUnavailableAlias(); + + void testSettingsAliasesModelDisplay(); + void testSettingsAliasesModel(); void testExposeSubgroup(); void testModifyingExposedGroup();