diff --git a/src/browser/BrowserOptionDialog.cpp b/src/browser/BrowserOptionDialog.cpp index a5bb921da..56c6cceb8 100644 --- a/src/browser/BrowserOptionDialog.cpp +++ b/src/browser/BrowserOptionDialog.cpp @@ -68,6 +68,10 @@ BrowserOptionDialog::BrowserOptionDialog(QWidget* parent) connect(m_ui->useCustomProxy, SIGNAL(toggled(bool)), m_ui->customProxyLocationBrowseButton, SLOT(setEnabled(bool))); connect(m_ui->customProxyLocationBrowseButton, SIGNAL(clicked()), this, SLOT(showProxyLocationFileDialog())); +#ifndef Q_OS_LINUX + m_ui->snapWarningLabel->setVisible(false); +#endif + #ifdef Q_OS_WIN // Brave uses Chrome's registry settings m_ui->braveSupport->setHidden(true); diff --git a/src/browser/BrowserOptionDialog.ui b/src/browser/BrowserOptionDialog.ui index 638c400aa..84fc5bdbf 100755 --- a/src/browser/BrowserOptionDialog.ui +++ b/src/browser/BrowserOptionDialog.ui @@ -60,7 +60,7 @@ - + Browsers installed as snaps are currently not supported. diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp index c00229fd8..e548b370c 100644 --- a/src/browser/BrowserService.cpp +++ b/src/browser/BrowserService.cpp @@ -380,7 +380,7 @@ QJsonArray BrowserService::findMatchingEntries(const QString& id, // Check entries for authorization QList pwEntriesToConfirm; QList pwEntries; - for (auto* entry : searchEntries(url, keyList)) { + for (auto* entry : searchEntries(url, submitUrl, keyList)) { if (entry->customData()->contains(BrowserService::OPTION_HIDE_ENTRY) && entry->customData()->value(BrowserService::OPTION_HIDE_ENTRY) == "true") { continue; @@ -583,7 +583,7 @@ BrowserService::ReturnValue BrowserService::updateEntry(const QString& id, } QList -BrowserService::searchEntries(const QSharedPointer& db, const QString& hostname, const QString& url) +BrowserService::searchEntries(const QSharedPointer& db, const QString& url, const QString& submitUrl) { QList entries; auto* rootGroup = db->rootGroup(); @@ -601,19 +601,17 @@ BrowserService::searchEntries(const QSharedPointer& db, const QString& continue; } - auto domain = baseDomain(hostname); - // Search for additional URL's starting with KP2A_URL if (entry->attributes()->keys().contains(ADDITIONAL_URL)) { for (const auto& key : entry->attributes()->keys()) { - if (key.startsWith(ADDITIONAL_URL) && handleURL(entry->attributes()->value(key), domain, url)) { + if (key.startsWith(ADDITIONAL_URL) && handleURL(entry->attributes()->value(key), url, submitUrl)) { entries.append(entry); continue; } } } - if (!handleURL(entry->url(), domain, url)) { + if (!handleURL(entry->url(), url, submitUrl)) { continue; } @@ -624,7 +622,7 @@ BrowserService::searchEntries(const QSharedPointer& db, const QString& return entries; } -QList BrowserService::searchEntries(const QString& url, const StringPairList& keyList) +QList BrowserService::searchEntries(const QString& url, const QString& submitUrl, const StringPairList& keyList) { // Check if database is connected with KeePassXC-Browser auto databaseConnected = [&](const QSharedPointer& db) { @@ -661,7 +659,7 @@ QList BrowserService::searchEntries(const QString& url, const StringPair QList entries; do { for (const auto& db : databases) { - entries << searchEntries(db, hostname, url); + entries << searchEntries(db, url, submitUrl); } } while (entries.isEmpty() && removeFirstDomain(hostname)); @@ -999,7 +997,7 @@ bool BrowserService::removeFirstDomain(QString& hostname) return false; } -bool BrowserService::handleURL(const QString& entryUrl, const QString& hostname, const QString& url) +bool BrowserService::handleURL(const QString& entryUrl, const QString& url, const QString& submitUrl) { if (entryUrl.isEmpty()) { return false; @@ -1016,32 +1014,36 @@ bool BrowserService::handleURL(const QString& entryUrl, const QString& hostname, } } + // Make a direct compare if a local file is used + if (url.contains("file://")) { + return entryUrl == submitUrl; + } + // URL host validation fails - if (browserSettings()->matchUrlScheme() && entryQUrl.host().isEmpty()) { + if (entryQUrl.host().isEmpty()) { return false; } // Match port, if used - QUrl qUrl(url); - if (entryQUrl.port() > 0 && entryQUrl.port() != qUrl.port()) { + QUrl siteQUrl(url); + if (entryQUrl.port() > 0 && entryQUrl.port() != siteQUrl.port()) { return false; } // Match scheme if (browserSettings()->matchUrlScheme() && !entryQUrl.scheme().isEmpty() - && entryQUrl.scheme().compare(qUrl.scheme()) != 0) { + && entryQUrl.scheme().compare(siteQUrl.scheme()) != 0) { return false; } // Check for illegal characters QRegularExpression re("[<>\\^`{|}]"); - auto match = re.match(entryUrl); - if (match.hasMatch()) { + if (re.match(entryUrl).hasMatch()) { return false; } // Filter to match hostname in URL field - if (entryQUrl.host().endsWith(hostname)) { + if (siteQUrl.host().endsWith(entryQUrl.host())) { return true; } diff --git a/src/browser/BrowserService.h b/src/browser/BrowserService.h index cb20ecbfb..6990eeda7 100644 --- a/src/browser/BrowserService.h +++ b/src/browser/BrowserService.h @@ -63,8 +63,8 @@ public: const QString& group, const QString& groupUuid, const QSharedPointer& selectedDb = {}); - QList searchEntries(const QSharedPointer& db, const QString& hostname, const QString& url); - QList searchEntries(const QString& url, const StringPairList& keyList); + QList searchEntries(const QSharedPointer& db, const QString& url, const QString& submitUrl); + QList searchEntries(const QString& url, const QString& submitUrl, const StringPairList& keyList); void convertAttributesToCustomData(const QSharedPointer& currentDb = {}); public: @@ -130,7 +130,7 @@ private: sortPriority(const Entry* entry, const QString& host, const QString& submitUrl, const QString& baseSubmitUrl) const; bool schemeFound(const QString& url); bool removeFirstDomain(QString& hostname); - bool handleURL(const QString& entryUrl, const QString& hostname, const QString& url); + bool handleURL(const QString& entryUrl, const QString& url, const QString& submitUrl); QString baseDomain(const QString& hostname) const; QSharedPointer getDatabase(); QSharedPointer selectedDatabase(); diff --git a/src/core/Bootstrap.cpp b/src/core/Bootstrap.cpp index 204942f4d..46aff92d5 100644 --- a/src/core/Bootstrap.cpp +++ b/src/core/Bootstrap.cpp @@ -106,7 +106,11 @@ namespace Bootstrap { // start minimized if configured if (config()->get("GUI/MinimizeOnStartup").toBool()) { +#ifdef Q_OS_WIN mainWindow.showMinimized(); +#else + mainWindow.hideWindow(); +#endif } else { mainWindow.bringToFront(); } diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 4cccd6d53..066abc4e3 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -42,7 +42,6 @@ Database::Database() : m_metadata(new Metadata(this)) , m_data() , m_rootGroup(nullptr) - , m_timer(new QTimer(this)) , m_fileWatcher(new FileWatcher(this)) , m_emitModified(false) , m_uuid(QUuid::createUuid()) @@ -50,12 +49,12 @@ Database::Database() setRootGroup(new Group()); rootGroup()->setUuid(QUuid::createUuid()); rootGroup()->setName(tr("Root", "Root group name")); - m_timer->setSingleShot(true); + m_modifiedTimer.setSingleShot(true); s_uuidMap.insert(m_uuid, this); connect(m_metadata, SIGNAL(metadataModified()), SLOT(markAsModified())); - connect(m_timer, SIGNAL(timeout()), SIGNAL(databaseModified())); + connect(&m_modifiedTimer, SIGNAL(timeout()), SIGNAL(databaseModified())); connect(this, SIGNAL(databaseOpened()), SLOT(updateCommonUsernames())); connect(this, SIGNAL(databaseSaved()), SLOT(updateCommonUsernames())); connect(m_fileWatcher, SIGNAL(fileChanged()), SIGNAL(databaseFileChanged())); @@ -229,6 +228,7 @@ bool Database::saveAs(const QString& filePath, QString* error, bool atomic, bool auto& canonicalFilePath = QFileInfo::exists(filePath) ? QFileInfo(filePath).canonicalFilePath() : filePath; bool ok = performSave(canonicalFilePath, error, atomic, backup); if (ok) { + markAsClean(); setFilePath(filePath); m_fileWatcher->start(canonicalFilePath, 30, 1); } else { @@ -343,7 +343,6 @@ bool Database::writeDatabase(QIODevice* device, QString* error) return false; } - markAsClean(); return true; } @@ -832,7 +831,7 @@ void Database::emptyRecycleBin() void Database::setEmitModified(bool value) { if (m_emitModified && !value) { - m_timer->stop(); + m_modifiedTimer.stop(); } m_emitModified = value; @@ -846,8 +845,9 @@ bool Database::isModified() const void Database::markAsModified() { m_modified = true; - if (m_emitModified) { - startModifiedTimer(); + if (m_emitModified && !m_modifiedTimer.isActive()) { + // Small time delay prevents numerous consecutive saves due to repeated signals + m_modifiedTimer.start(150); } } @@ -855,6 +855,7 @@ void Database::markAsClean() { bool emitSignal = m_modified; m_modified = false; + m_modifiedTimer.stop(); if (emitSignal) { emit databaseSaved(); } @@ -869,18 +870,6 @@ Database* Database::databaseByUuid(const QUuid& uuid) return s_uuidMap.value(uuid, nullptr); } -void Database::startModifiedTimer() -{ - if (!m_emitModified) { - return; - } - - if (m_timer->isActive()) { - m_timer->stop(); - } - m_timer->start(150); -} - QSharedPointer Database::key() const { return m_data.key; diff --git a/src/core/Database.h b/src/core/Database.h index d5f2092b2..d3d88e7d2 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -21,9 +21,9 @@ #include #include -#include #include #include +#include #include "config-keepassx.h" #include "crypto/kdf/AesKdf.h" @@ -37,7 +37,6 @@ enum class EntryReferenceType; class FileWatcher; class Group; class Metadata; -class QTimer; class QIODevice; struct DeletedObject @@ -155,9 +154,6 @@ signals: void databaseDiscarded(); void databaseFileChanged(); -private slots: - void startModifiedTimer(); - private: struct DatabaseData { @@ -211,7 +207,7 @@ private: DatabaseData m_data; QPointer m_rootGroup; QList m_deletedObjects; - QPointer m_timer; + QTimer m_modifiedTimer; QPointer m_fileWatcher; bool m_initialized = false; bool m_modified = false; diff --git a/src/core/FileWatcher.cpp b/src/core/FileWatcher.cpp index 0d1def31a..fb8e95128 100644 --- a/src/core/FileWatcher.cpp +++ b/src/core/FileWatcher.cpp @@ -35,11 +35,10 @@ namespace FileWatcher::FileWatcher(QObject* parent) : QObject(parent) - , m_ignoreFileChange(false) { - connect(&m_fileWatcher, SIGNAL(fileChanged(QString)), SLOT(onWatchedFileChanged())); + connect(&m_fileWatcher, SIGNAL(fileChanged(QString)), SLOT(checkFileChanged())); + connect(&m_fileChecksumTimer, SIGNAL(timeout()), SLOT(checkFileChanged())); connect(&m_fileChangeDelayTimer, SIGNAL(timeout()), SIGNAL(fileChanged())); - connect(&m_fileChecksumTimer, SIGNAL(timeout()), SLOT(checkFileChecksum())); m_fileChangeDelayTimer.setSingleShot(true); m_fileIgnoreDelayTimer.setSingleShot(true); } @@ -101,17 +100,6 @@ void FileWatcher::resume() } } -void FileWatcher::onWatchedFileChanged() -{ - // Don't notify if we are ignoring events or already started a notification chain - if (shouldIgnoreChanges()) { - return; - } - - m_fileChecksum = calculateChecksum(); - m_fileChangeDelayTimer.start(0); -} - bool FileWatcher::shouldIgnoreChanges() { return m_filePath.isEmpty() || m_ignoreFileChange || m_fileIgnoreDelayTimer.isActive() @@ -123,15 +111,23 @@ bool FileWatcher::hasSameFileChecksum() return calculateChecksum() == m_fileChecksum; } -void FileWatcher::checkFileChecksum() +void FileWatcher::checkFileChanged() { if (shouldIgnoreChanges()) { return; } - if (!hasSameFileChecksum()) { - onWatchedFileChanged(); + // Prevent reentrance + m_ignoreFileChange = true; + + // Only trigger the change notice if there is a checksum mismatch + auto checksum = calculateChecksum(); + if (checksum != m_fileChecksum) { + m_fileChecksum = checksum; + m_fileChangeDelayTimer.start(0); } + + m_ignoreFileChange = false; } QByteArray FileWatcher::calculateChecksum() diff --git a/src/core/FileWatcher.h b/src/core/FileWatcher.h index fea05fc84..9b55badc1 100644 --- a/src/core/FileWatcher.h +++ b/src/core/FileWatcher.h @@ -43,8 +43,7 @@ public slots: void resume(); private slots: - void onWatchedFileChanged(); - void checkFileChecksum(); + void checkFileChanged(); private: QByteArray calculateChecksum(); @@ -56,8 +55,8 @@ private: QTimer m_fileChangeDelayTimer; QTimer m_fileIgnoreDelayTimer; QTimer m_fileChecksumTimer; - int m_fileChecksumSizeBytes; - bool m_ignoreFileChange; + int m_fileChecksumSizeBytes = -1; + bool m_ignoreFileChange = false; }; class BulkFileWatcher : public QObject diff --git a/src/fdosecrets/objects/Collection.cpp b/src/fdosecrets/objects/Collection.cpp index c826c1db0..30a8f2d0c 100644 --- a/src/fdosecrets/objects/Collection.cpp +++ b/src/fdosecrets/objects/Collection.cpp @@ -328,7 +328,11 @@ namespace FdoSecrets // when creation finishes in backend, we will already have item item = m_entryToItem.value(entry, nullptr); - Q_ASSERT(item); + + if (!item) { + // may happen if entry somehow ends up in recycle bin + return DBusReturn<>::Error(QStringLiteral(DBUS_ERROR_SECRET_NO_SUCH_OBJECT)); + } } ret = item->setProperties(properties); @@ -439,7 +443,7 @@ namespace FdoSecrets auto newUuid = FdoSecrets::settings()->exposedGroup(m_backend->database()); auto newGroup = m_backend->database()->rootGroup()->findGroupByUuid(newUuid); - if (!newGroup) { + if (!newGroup || inRecycleBin(newGroup)) { // no exposed group, delete self doDelete(); return; @@ -451,10 +455,11 @@ namespace FdoSecrets m_exposedGroup = newGroup; // Attach signal to update exposed group settings if the group was removed. + // // The lifetime of the connection is bound to the database object, because - // in Database::~Database, groups are also deleted, but we don't want to - // trigger this. - // This rely on the fact that QObject disconnects signals BEFORE deleting + // in Database::~Database, groups are also deleted as children, but we don't + // want to trigger this. + // This works because the fact that QObject disconnects signals BEFORE deleting // children. QPointer db = m_backend->database().data(); connect(m_exposedGroup.data(), &Group::groupAboutToRemove, db, [db](Group* toBeRemoved) { @@ -468,6 +473,13 @@ namespace FdoSecrets FdoSecrets::settings()->setExposedGroup(db, {}); } }); + // Another possibility is the group being moved to recycle bin. + connect(m_exposedGroup.data(), &Group::groupModified, this, [this]() { + if (inRecycleBin(m_exposedGroup->parentGroup())) { + // reset the exposed group to none + FdoSecrets::settings()->setExposedGroup(m_backend->database().data(), {}); + } + }); // Monitor exposed group settings connect(m_backend->database()->metadata()->customData(), &CustomData::customDataModified, this, [this]() { @@ -646,17 +658,21 @@ namespace FdoSecrets { Q_ASSERT(m_backend); - if (!m_backend->database()->metadata()->recycleBin()) { + if (!group) { + // just to be safe + return true; + } + + if (!m_backend->database()->metadata()) { return false; } - while (group) { - if (group->uuid() == m_backend->database()->metadata()->recycleBin()->uuid()) { - return true; - } - group = group->parentGroup(); + auto recycleBin = m_backend->database()->metadata()->recycleBin(); + if (!recycleBin) { + return false; } - return false; + + return group->uuid() == recycleBin->uuid() || group->isRecycled(); } bool Collection::inRecycleBin(Entry* entry) const diff --git a/src/fdosecrets/objects/Collection.h b/src/fdosecrets/objects/Collection.h index de9db3a49..06d45a50e 100644 --- a/src/fdosecrets/objects/Collection.h +++ b/src/fdosecrets/objects/Collection.h @@ -74,6 +74,11 @@ namespace FdoSecrets public: DBusReturn setProperties(const QVariantMap& properties); + bool isValid() const + { + return backend(); + } + DBusReturn removeAlias(QString alias); DBusReturn addAlias(QString alias); const QSet aliases() const; @@ -106,6 +111,7 @@ namespace FdoSecrets private slots: void onDatabaseLockChanged(); void onDatabaseExposedGroupChanged(); + // force reload info from backend, potentially delete self void reloadBackend(); private: diff --git a/src/fdosecrets/objects/Service.cpp b/src/fdosecrets/objects/Service.cpp index 6bca1f12c..eeded79ba 100644 --- a/src/fdosecrets/objects/Service.cpp +++ b/src/fdosecrets/objects/Service.cpp @@ -93,7 +93,24 @@ namespace FdoSecrets void Service::onDatabaseTabOpened(DatabaseWidget* dbWidget, bool emitSignal) { + // The Collection will monitor the database's exposed group. + // When the Collection finds that no exposed group, it will delete itself. + // Thus the service also needs to monitor it and recreate the collection if the user changes + // from no exposed to exposed something. + if (!dbWidget->isLocked()) { + monitorDatabaseExposedGroup(dbWidget); + } + connect(dbWidget, &DatabaseWidget::databaseUnlocked, this, [this, dbWidget]() { + monitorDatabaseExposedGroup(dbWidget); + }); + auto coll = new Collection(this, dbWidget); + // Creation may fail if the database is not exposed. + // This is okay, because we monitor the expose settings above + if (!coll->isValid()) { + coll->deleteLater(); + return; + } m_collections << coll; m_dbToCollection[dbWidget] = coll; @@ -127,15 +144,6 @@ namespace FdoSecrets emit collectionDeleted(coll); }); - // a special case: the database changed from no expose to expose something. - // in this case, there is no collection out there monitoring it, so create a new collection - if (!dbWidget->isLocked()) { - monitorDatabaseExposedGroup(dbWidget); - } - connect(dbWidget, &DatabaseWidget::databaseUnlocked, this, [this, dbWidget]() { - monitorDatabaseExposedGroup(dbWidget); - }); - if (emitSignal) { emit collectionCreated(coll); } diff --git a/src/fdosecrets/widgets/DatabaseSettingsWidgetFdoSecrets.cpp b/src/fdosecrets/widgets/DatabaseSettingsWidgetFdoSecrets.cpp index fadd01542..16a780de7 100644 --- a/src/fdosecrets/widgets/DatabaseSettingsWidgetFdoSecrets.cpp +++ b/src/fdosecrets/widgets/DatabaseSettingsWidgetFdoSecrets.cpp @@ -85,7 +85,7 @@ protected: // can not call mapFromSource, which internally calls filterAcceptsRow auto group = groupFromSourceIndex(source_idx); - return group->uuid() != recycleBin->uuid(); + return group && !group->isRecycled() && group->uuid() != recycleBin->uuid(); } }; @@ -118,8 +118,13 @@ void DatabaseSettingsWidgetFdoSecrets::loadSettings(QSharedPointer db) m_model.reset(new GroupModelNoRecycle(m_db.data())); m_ui->selectGroup->setModel(m_model.data()); + Group* recycleBin = nullptr; + if (m_db->metadata() && m_db->metadata()->recycleBin()) { + recycleBin = m_db->metadata()->recycleBin(); + } + auto group = m_db->rootGroup()->findGroupByUuid(FdoSecrets::settings()->exposedGroup(m_db)); - if (!group) { + if (!group || group->isRecycled() || (recycleBin && group->uuid() == recycleBin->uuid())) { m_ui->radioDonotExpose->setChecked(true); } else { auto idx = m_model->indexFromGroup(group); diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index f90fd37d1..c610a773d 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -204,9 +204,14 @@ void DatabaseOpenWidget::openDatabase() m_db.reset(new Database()); QString error; + QApplication::setOverrideCursor(QCursor(Qt::WaitCursor)); + m_ui->passwordFormFrame->setEnabled(false); + QCoreApplication::processEvents(); bool ok = m_db->open(m_filename, masterKey, &error, false); QApplication::restoreOverrideCursor(); + m_ui->passwordFormFrame->setEnabled(true); + if (!ok) { if (m_ui->editPassword->text().isEmpty() && !m_retryUnlockWithEmptyPassword) { QScopedPointer msgBox(new QMessageBox(this)); diff --git a/src/gui/DatabaseOpenWidget.ui b/src/gui/DatabaseOpenWidget.ui index 14a1337c6..60b2feadc 100644 --- a/src/gui/DatabaseOpenWidget.ui +++ b/src/gui/DatabaseOpenWidget.ui @@ -132,7 +132,7 @@ 15 - + 400 diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index 29942571c..5c4bf39ac 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -275,6 +275,11 @@ bool DatabaseWidget::isEntryEditActive() const return currentWidget() == m_editEntryWidget; } +bool DatabaseWidget::isGroupEditActive() const +{ + return currentWidget() == m_editGroupWidget; +} + bool DatabaseWidget::isEditWidgetModified() const { if (currentWidget() == m_editEntryWidget) { @@ -387,6 +392,8 @@ void DatabaseWidget::createEntry() void DatabaseWidget::replaceDatabase(QSharedPointer db) { + Q_ASSERT(!isEntryEditActive() && !isGroupEditActive()); + // Save off new parent UUID which will be valid when creating a new entry QUuid newParentUuid; if (m_newParent) { @@ -1370,7 +1377,7 @@ bool DatabaseWidget::lock() if (m_db->isModified()) { bool saved = false; // Attempt to save on exit, but don't block locking if it fails - if (config()->get("AutoSaveOnExit").toBool()) { + if (config()->get("AutoSaveOnExit").toBool() || config()->get("AutoSaveAfterEveryChange").toBool()) { saved = save(); } @@ -1421,7 +1428,8 @@ bool DatabaseWidget::lock() void DatabaseWidget::reloadDatabaseFile() { - if (!m_db || isLocked()) { + // Ignore reload if we are locked or currently editing an entry or group + if (!m_db || isLocked() || isEntryEditActive() || isGroupEditActive()) { return; } @@ -1441,6 +1449,11 @@ void DatabaseWidget::reloadDatabaseFile() } } + // Lock out interactions + m_entryView->setDisabled(true); + m_groupView->setDisabled(true); + QApplication::processEvents(); + QString error; auto db = QSharedPointer::create(m_db->filePath()); if (db->open(database()->key(), &error)) { @@ -1480,6 +1493,10 @@ void DatabaseWidget::reloadDatabaseFile() // Mark db as modified since existing data may differ from file or file was deleted m_db->markAsModified(); } + + // Return control + m_entryView->setDisabled(false); + m_groupView->setDisabled(false); } int DatabaseWidget::numberOfSelectedEntries() const @@ -1620,11 +1637,20 @@ bool DatabaseWidget::save() m_blockAutoSave = true; ++m_saveAttempts; - // TODO: Make this async, but lock out the database widget to prevent re-entrance + // TODO: Make this async + // Lock out interactions + m_entryView->setDisabled(true); + m_groupView->setDisabled(true); + QApplication::processEvents(); + bool useAtomicSaves = config()->get("UseAtomicSaves", true).toBool(); QString errorMessage; bool ok = m_db->save(&errorMessage, useAtomicSaves, config()->get("BackupBeforeSave").toBool()); + // Return control + m_entryView->setDisabled(false); + m_groupView->setDisabled(false); + if (ok) { m_saveAttempts = 0; m_blockAutoSave = false; diff --git a/src/gui/DatabaseWidget.h b/src/gui/DatabaseWidget.h index 6f40c65c5..9f0c5c976 100644 --- a/src/gui/DatabaseWidget.h +++ b/src/gui/DatabaseWidget.h @@ -81,6 +81,7 @@ public: bool isLocked() const; bool isSearchActive() const; bool isEntryEditActive() const; + bool isGroupEditActive() const; QString getCurrentSearch(); void refreshSearch(); diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index 3fa75e3b8..b80a4850d 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -201,7 +201,7 @@ void EditEntryWidget::setupAdvanced() connect(m_advancedUi->editAttributeButton, SIGNAL(clicked()), SLOT(editCurrentAttribute())); connect(m_advancedUi->removeAttributeButton, SIGNAL(clicked()), SLOT(removeCurrentAttribute())); connect(m_advancedUi->protectAttributeButton, SIGNAL(toggled(bool)), SLOT(protectCurrentAttribute(bool))); - connect(m_advancedUi->revealAttributeButton, SIGNAL(clicked(bool)), SLOT(revealCurrentAttribute())); + connect(m_advancedUi->revealAttributeButton, SIGNAL(clicked(bool)), SLOT(toggleCurrentAttributeVisibility())); connect(m_advancedUi->attributesView->selectionModel(), SIGNAL(currentChanged(QModelIndex,QModelIndex)), SLOT(updateCurrentAttribute())); @@ -1297,6 +1297,7 @@ void EditEntryWidget::displayAttribute(QModelIndex index, bool showProtected) // Block signals to prevent modified being set m_advancedUi->protectAttributeButton->blockSignals(true); m_advancedUi->attributesEdit->blockSignals(true); + m_advancedUi->revealAttributeButton->setText(tr("Reveal")); if (index.isValid()) { QString key = m_attributesModel->keyByIndex(index); @@ -1348,7 +1349,7 @@ void EditEntryWidget::protectCurrentAttribute(bool state) } } -void EditEntryWidget::revealCurrentAttribute() +void EditEntryWidget::toggleCurrentAttributeVisibility() { if (!m_advancedUi->attributesEdit->isEnabled()) { QModelIndex index = m_advancedUi->attributesView->currentIndex(); @@ -1359,6 +1360,10 @@ void EditEntryWidget::revealCurrentAttribute() m_advancedUi->attributesEdit->setEnabled(true); m_advancedUi->attributesEdit->blockSignals(oldBlockSignals); } + m_advancedUi->revealAttributeButton->setText(tr("Hide")); + } else { + protectCurrentAttribute(true); + m_advancedUi->revealAttributeButton->setText(tr("Reveal")); } } diff --git a/src/gui/entry/EditEntryWidget.h b/src/gui/entry/EditEntryWidget.h index 39b5fc5d9..300220cd0 100644 --- a/src/gui/entry/EditEntryWidget.h +++ b/src/gui/entry/EditEntryWidget.h @@ -93,7 +93,7 @@ private slots: void removeCurrentAttribute(); void updateCurrentAttribute(); void protectCurrentAttribute(bool state); - void revealCurrentAttribute(); + void toggleCurrentAttributeVisibility(); void updateAutoTypeEnabled(); void openAutotypeHelp(); void insertAutoTypeAssoc(); diff --git a/src/gui/widgets/ElidedLabel.cpp b/src/gui/widgets/ElidedLabel.cpp index bc2771764..749f075c8 100644 --- a/src/gui/widgets/ElidedLabel.cpp +++ b/src/gui/widgets/ElidedLabel.cpp @@ -105,8 +105,10 @@ void ElidedLabel::updateElidedText() const QFontMetrics metrix(font()); displayText = metrix.elidedText(m_rawText, m_elideMode, width() - 2); } - setText(m_url.isEmpty() ? displayText : htmlLinkTemplate.arg(m_url, displayText)); - setOpenExternalLinks(!m_url.isEmpty()); + + bool hasUrl = !m_url.isEmpty(); + setText(hasUrl ? htmlLinkTemplate.arg(m_url.toHtmlEscaped(), displayText) : displayText); + setOpenExternalLinks(!hasUrl); } void ElidedLabel::resizeEvent(QResizeEvent* event) diff --git a/tests/TestBrowser.cpp b/tests/TestBrowser.cpp index 818dfaebd..938a7e4e5 100644 --- a/tests/TestBrowser.cpp +++ b/tests/TestBrowser.cpp @@ -179,29 +179,22 @@ void TestBrowser::testSearchEntries() auto db = QSharedPointer::create(); auto* root = db->rootGroup(); - QList urls; - urls.push_back("https://github.com/login_page"); - urls.push_back("https://github.com/login"); - urls.push_back("https://github.com/"); - urls.push_back("github.com/login"); - urls.push_back("http://github.com"); - urls.push_back("http://github.com/login"); - urls.push_back("github.com"); - urls.push_back("github.com/login"); - urls.push_back("https://github"); // Invalid URL - urls.push_back("github.com"); + QStringList urls = {"https://github.com/login_page", + "https://github.com/login", + "https://github.com/", + "github.com/login", + "http://github.com", + "http://github.com/login", + "github.com", + "github.com/login", + "https://github", // Invalid URL + "github.com"}; - for (int i = 0; i < urls.length(); ++i) { - auto entry = new Entry(); - entry->setGroup(root); - entry->beginUpdate(); - entry->setUrl(urls[i]); - entry->setUsername(QString("User %1").arg(i)); - entry->endUpdate(); - } + createEntries(urls, root); browserSettings()->setMatchUrlScheme(false); - auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url + auto result = + m_browserService->searchEntries(db, "https://github.com", "https://github.com/session"); // db, url, submitUrl QCOMPARE(result.length(), 9); QCOMPARE(result[0]->url(), QString("https://github.com/login_page")); @@ -213,7 +206,7 @@ void TestBrowser::testSearchEntries() // With matching there should be only 3 results + 4 without a scheme browserSettings()->setMatchUrlScheme(true); - result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url + result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session"); QCOMPARE(result.length(), 7); QCOMPARE(result[0]->url(), QString("https://github.com/login_page")); QCOMPARE(result[1]->url(), QString("https://github.com/login")); @@ -226,22 +219,13 @@ void TestBrowser::testSearchEntriesWithPort() auto db = QSharedPointer::create(); auto* root = db->rootGroup(); - QList urls; - urls.push_back("http://127.0.0.1:443"); - urls.push_back("http://127.0.0.1:80"); + QStringList urls = {"http://127.0.0.1:443", "http://127.0.0.1:80"}; - for (int i = 0; i < urls.length(); ++i) { - auto entry = new Entry(); - entry->setGroup(root); - entry->beginUpdate(); - entry->setUrl(urls[i]); - entry->setUsername(QString("User %1").arg(i)); - entry->endUpdate(); - } + createEntries(urls, root); - auto result = m_browserService->searchEntries(db, "127.0.0.1", "http://127.0.0.1:443"); // db, hostname, url + auto result = m_browserService->searchEntries(db, "http://127.0.0.1:443", "http://127.0.0.1"); QCOMPARE(result.length(), 1); - QCOMPARE(result[0]->url(), urls[0]); + QCOMPARE(result[0]->url(), QString("http://127.0.0.1:443")); } void TestBrowser::testSearchEntriesWithAdditionalURLs() @@ -249,70 +233,55 @@ void TestBrowser::testSearchEntriesWithAdditionalURLs() auto db = QSharedPointer::create(); auto* root = db->rootGroup(); - QList entries; - QList urls; - urls.push_back("https://github.com/"); - urls.push_back("https://www.example.com"); - urls.push_back("http://domain.com"); + QStringList urls = {"https://github.com/", "https://www.example.com", "http://domain.com"}; - for (int i = 0; i < urls.length(); ++i) { - auto entry = new Entry(); - entry->setGroup(root); - entry->beginUpdate(); - entry->setUrl(urls[i]); - entry->setUsername(QString("User %1").arg(i)); - entry->endUpdate(); - entries.push_back(entry); - } + auto entries = createEntries(urls, root); // Add an additional URL to the first entry entries.first()->attributes()->set(BrowserService::ADDITIONAL_URL, "https://keepassxc.org"); - auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url + auto result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session"); QCOMPARE(result.length(), 1); - QCOMPARE(result[0]->url(), urls[0]); + QCOMPARE(result[0]->url(), QString("https://github.com/")); // Search the additional URL. It should return the same entry - auto additionalResult = m_browserService->searchEntries(db, "keepassxc.org", "https://keepassxc.org"); + auto additionalResult = m_browserService->searchEntries(db, "https://keepassxc.org", "https://keepassxc.org"); QCOMPARE(additionalResult.length(), 1); - QCOMPARE(additionalResult[0]->url(), urls[0]); + QCOMPARE(additionalResult[0]->url(), QString("https://github.com/")); } void TestBrowser::testInvalidEntries() { auto db = QSharedPointer::create(); auto* root = db->rootGroup(); + const QString url("https://github.com"); + const QString submitUrl("https://github.com/session"); - QList urls; - urls.push_back("https://github.com/login"); - urls.push_back("https:///github.com/"); // Extra '/' - urls.push_back("http://github.com/**//*"); - urls.push_back("http://*.github.com/login"); - urls.push_back("//github.com"); // fromUserInput() corrects this one. - urls.push_back("github.com/{}<>"); + QStringList urls = { + "https://github.com/login", + "https:///github.com/", // Extra '/' + "http://github.com/**//*", + "http://*.github.com/login", + "//github.com", // fromUserInput() corrects this one. + "github.com/{}<>", + "http:/example.com", + }; - for (int i = 0; i < urls.length(); ++i) { - auto entry = new Entry(); - entry->setGroup(root); - entry->beginUpdate(); - entry->setUrl(urls[i]); - entry->setUsername(QString("User %1").arg(i)); - entry->endUpdate(); - } + createEntries(urls, root); browserSettings()->setMatchUrlScheme(true); - auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url + auto result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session"); QCOMPARE(result.length(), 2); QCOMPARE(result[0]->url(), QString("https://github.com/login")); QCOMPARE(result[1]->url(), QString("//github.com")); // Test the URL's directly - QCOMPARE(m_browserService->handleURL(urls[0], "github.com", "https://github.com"), true); - QCOMPARE(m_browserService->handleURL(urls[1], "github.com", "https://github.com"), false); - QCOMPARE(m_browserService->handleURL(urls[2], "github.com", "https://github.com"), false); - QCOMPARE(m_browserService->handleURL(urls[3], "github.com", "https://github.com"), false); - QCOMPARE(m_browserService->handleURL(urls[4], "github.com", "https://github.com"), true); - QCOMPARE(m_browserService->handleURL(urls[5], "github.com", "https://github.com"), false); + QCOMPARE(m_browserService->handleURL(urls[0], url, submitUrl), true); + QCOMPARE(m_browserService->handleURL(urls[1], url, submitUrl), false); + QCOMPARE(m_browserService->handleURL(urls[2], url, submitUrl), false); + QCOMPARE(m_browserService->handleURL(urls[3], url, submitUrl), false); + QCOMPARE(m_browserService->handleURL(urls[4], url, submitUrl), true); + QCOMPARE(m_browserService->handleURL(urls[5], url, submitUrl), false); } void TestBrowser::testSubdomainsAndPaths() @@ -320,44 +289,74 @@ void TestBrowser::testSubdomainsAndPaths() auto db = QSharedPointer::create(); auto* root = db->rootGroup(); - QList urls; - urls.push_back("https://www.github.com/login/page.xml"); - urls.push_back("https://login.github.com/"); - urls.push_back("https://github.com"); - urls.push_back("http://www.github.com"); - urls.push_back("http://login.github.com/pathtonowhere"); - urls.push_back(".github.com"); // Invalid URL - urls.push_back("www.github.com/"); - urls.push_back("https://github"); // Invalid URL + QStringList urls = { + "https://www.github.com/login/page.xml", + "https://login.github.com/", + "https://github.com", + "http://www.github.com", + "http://login.github.com/pathtonowhere", + ".github.com", // Invalid URL + "www.github.com/", + "https://github" // Invalid URL + }; - for (int i = 0; i < urls.length(); ++i) { - auto entry = new Entry(); - entry->setGroup(root); - entry->beginUpdate(); - entry->setUrl(urls[i]); - entry->setUsername(QString("User %1").arg(i)); - entry->endUpdate(); - } + createEntries(urls, root); browserSettings()->setMatchUrlScheme(false); - auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url + auto result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session"); + QCOMPARE(result.length(), 1); + QCOMPARE(result[0]->url(), QString("https://github.com")); - QCOMPARE(result.length(), 6); - QCOMPARE(result[0]->url(), urls[0]); - QCOMPARE(result[1]->url(), urls[1]); - QCOMPARE(result[2]->url(), urls[2]); - QCOMPARE(result[3]->url(), urls[3]); - QCOMPARE(result[4]->url(), urls[4]); - QCOMPARE(result[5]->url(), urls[6]); - - // With matching there should be only 3 results - browserSettings()->setMatchUrlScheme(true); - result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url + // With www subdomain + result = m_browserService->searchEntries(db, "https://www.github.com", "https://www.github.com/session"); QCOMPARE(result.length(), 4); - QCOMPARE(result[0]->url(), urls[0]); - QCOMPARE(result[1]->url(), urls[1]); - QCOMPARE(result[2]->url(), urls[2]); - QCOMPARE(result[3]->url(), urls[6]); + QCOMPARE(result[0]->url(), QString("https://www.github.com/login/page.xml")); + QCOMPARE(result[1]->url(), QString("https://github.com")); // Accepts any subdomain + QCOMPARE(result[2]->url(), QString("http://www.github.com")); + QCOMPARE(result[3]->url(), QString("www.github.com/")); + + // With scheme matching there should be only 1 result + browserSettings()->setMatchUrlScheme(true); + result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session"); + QCOMPARE(result.length(), 1); + QCOMPARE(result[0]->url(), QString("https://github.com")); + + // Test site with subdomain in the site URL + QStringList entryURLs = { + "https://accounts.example.com", + "https://accounts.example.com/path", + "https://subdomain.example.com/", + "https://another.accounts.example.com/", + "https://another.subdomain.example.com/", + "https://example.com/", + "https://example" // Invalid URL + }; + + createEntries(entryURLs, root); + + result = m_browserService->searchEntries(db, "https://accounts.example.com", "https://accounts.example.com"); + QCOMPARE(result.length(), 3); + QCOMPARE(result[0]->url(), QString("https://accounts.example.com")); + QCOMPARE(result[1]->url(), QString("https://accounts.example.com/path")); + QCOMPARE(result[2]->url(), QString("https://example.com/")); // Accepts any subdomain + + result = m_browserService->searchEntries( + db, "https://another.accounts.example.com", "https://another.accounts.example.com"); + QCOMPARE(result.length(), 4); + QCOMPARE(result[0]->url(), + QString("https://accounts.example.com")); // Accepts any subdomain under accounts.example.com + QCOMPARE(result[1]->url(), QString("https://accounts.example.com/path")); + QCOMPARE(result[2]->url(), QString("https://another.accounts.example.com/")); + QCOMPARE(result[3]->url(), QString("https://example.com/")); // Accepts one or more subdomains + + // Test local files. It should be a direct match. + QStringList localFiles = {"file:///Users/testUser/tests/test.html"}; + + createEntries(localFiles, root); + + // With local files, url is always set to the file scheme + ://. Submit URL holds the actual URL. + result = m_browserService->searchEntries(db, "file://", "file:///Users/testUser/tests/test.html"); + QCOMPARE(result.length(), 1); } void TestBrowser::testSortEntries() @@ -365,28 +364,18 @@ void TestBrowser::testSortEntries() auto db = QSharedPointer::create(); auto* root = db->rootGroup(); - QList urls; - urls.push_back("https://github.com/login_page"); - urls.push_back("https://github.com/login"); - urls.push_back("https://github.com/"); - urls.push_back("github.com/login"); - urls.push_back("http://github.com"); - urls.push_back("http://github.com/login"); - urls.push_back("github.com"); - urls.push_back("github.com/login"); - urls.push_back("https://github"); // Invalid URL - urls.push_back("github.com"); + QStringList urls = {"https://github.com/login_page", + "https://github.com/login", + "https://github.com/", + "github.com/login", + "http://github.com", + "http://github.com/login", + "github.com", + "github.com/login", + "https://github", // Invalid URL + "github.com"}; - QList entries; - for (int i = 0; i < urls.length(); ++i) { - auto entry = new Entry(); - entry->setGroup(root); - entry->beginUpdate(); - entry->setUrl(urls[i]); - entry->setUsername(QString("User %1").arg(i)); - entry->endUpdate(); - entries.push_back(entry); - } + auto entries = createEntries(urls, root); browserSettings()->setBestMatchOnly(false); auto result = @@ -457,3 +446,19 @@ void TestBrowser::testGetDatabaseGroups() auto lastChild = lastChildren.at(0); QCOMPARE(lastChild.toObject()["name"].toString(), QString("group2_1_1")); } + +QList TestBrowser::createEntries(QStringList& urls, Group* root) const +{ + QList entries; + for (int i = 0; i < urls.length(); ++i) { + auto entry = new Entry(); + entry->setGroup(root); + entry->beginUpdate(); + entry->setUrl(urls[i]); + entry->setUsername(QString("User %1").arg(i)); + entry->endUpdate(); + entries.push_back(entry); + } + + return entries; +} \ No newline at end of file diff --git a/tests/TestBrowser.h b/tests/TestBrowser.h index 981c1642d..8b2dc3e3c 100644 --- a/tests/TestBrowser.h +++ b/tests/TestBrowser.h @@ -49,6 +49,8 @@ private slots: void testGetDatabaseGroups(); private: + QList createEntries(QStringList& urls, Group* root) const; + QScopedPointer m_browserAction; QScopedPointer m_browserService; };