From 7858430631d8c368b9ea20b6fee62eb35927c5b4 Mon Sep 17 00:00:00 2001 From: Carlo Bertoldi Date: Thu, 20 Aug 2020 10:23:08 +0200 Subject: [PATCH 01/15] Add a note for mac users in documentation * Add a custom style to make keyboard shortcuts readable when experimental keyboard display is made official. --- docs/styles/dark.css | 3 ++- docs/topics/KeyboardShortcuts.adoc | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/styles/dark.css b/docs/styles/dark.css index 2c5b50a29..a32c366f3 100644 --- a/docs/styles/dark.css +++ b/docs/styles/dark.css @@ -455,6 +455,7 @@ p{font-family: "Noto Sans",sans-serif !important} blockquote{color:var(--quotecolor) !important} .quoteblock{color:var(--textcolor)} code{color:var(--textcoloralt);background-color: var(--sidebarbackground) !important} +.keyseq{color:var(--textcoloralt);} /* Table styles */ @@ -531,4 +532,4 @@ a:hover {color: var(--linkhovercolor);} } .subtitle { font-size: 1.5em; -} \ No newline at end of file +} diff --git a/docs/topics/KeyboardShortcuts.adoc b/docs/topics/KeyboardShortcuts.adoc index 837fa9608..93baa303d 100644 --- a/docs/topics/KeyboardShortcuts.adoc +++ b/docs/topics/KeyboardShortcuts.adoc @@ -3,6 +3,8 @@ include::.sharedheader[] :imagesdir: ../images // tag::content[] +NOTE: On macOS please substitute `Ctrl` with `Cmd` (aka `⌘`). + [grid=rows, frame=none, width=75%] |=== |Action | Keyboard Shortcut @@ -31,6 +33,7 @@ include::.sharedheader[] |Hide Window | Ctrl + Shift + M |Select Next Database Tab | Ctrl + Tab ; Ctrl + PageDn |Select Previous Database Tab | Ctrl + Shift + Tab ; Ctrl + PageUp +|Select the nth database | Ctrl + n, where n is the number of the database tab |Toggle Passwords Hidden | Ctrl + Shift + C |Toggle Usernames Hidden | Ctrl + Shift + B |Focus Groups (edit if focused) | F1 From aedc45abd508cde810b4924eb2a7dd1123965a8f Mon Sep 17 00:00:00 2001 From: Reza Jelveh Date: Sat, 29 Aug 2020 20:47:26 +0800 Subject: [PATCH 02/15] The Database Open Dialog should use the window flag QT::Dialog Currently the Open Dialog does not behave like a dialog. In Unix it means that the EWHM hints are not set correctly therefore the window manager doesn't properly set the floating window style. It should also allow removing Mac/Windows/Unix custom conditional code. --- src/gui/DatabaseOpenDialog.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/gui/DatabaseOpenDialog.cpp b/src/gui/DatabaseOpenDialog.cpp index 5e6e41b7a..e7194b7e2 100644 --- a/src/gui/DatabaseOpenDialog.cpp +++ b/src/gui/DatabaseOpenDialog.cpp @@ -25,11 +25,7 @@ DatabaseOpenDialog::DatabaseOpenDialog(QWidget* parent) , m_view(new DatabaseOpenWidget(this)) { setWindowTitle(tr("Unlock Database - KeePassXC")); -#ifdef Q_OS_MACOS - setWindowFlags(windowFlags() | Qt::WindowStaysOnTopHint); -#else - setWindowFlags(windowFlags() | Qt::WindowStaysOnTopHint | Qt::ForeignWindow); -#endif + setWindowFlags(Qt::Dialog | Qt::WindowStaysOnTopHint); connect(m_view, SIGNAL(dialogFinished(bool)), this, SLOT(complete(bool))); auto* layout = new QVBoxLayout(); layout->setMargin(0); From 639e44e182408febb1ef5e0f0645c839f9787722 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Tue, 25 Aug 2020 23:41:02 -0400 Subject: [PATCH 03/15] Fix opening opvault on macos * Fixes #4069 and closes #5002 --- src/gui/DatabaseTabWidget.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index 34fe4db72..2683cecec 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -275,7 +275,7 @@ void DatabaseTabWidget::importKeePass1Database() void DatabaseTabWidget::importOpVaultDatabase() { -#ifdef Q_MACOS +#ifdef Q_OS_MACOS QString fileName = fileDialog()->getOpenFileName(this, tr("Open OPVault"), {}, "OPVault (*.opvault)"); #else QString fileName = fileDialog()->getExistingDirectory(this, tr("Open OPVault")); From f17fce9461521e1927c07da799917e520ab7538f Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sun, 23 Aug 2020 12:21:59 -0400 Subject: [PATCH 04/15] Fix Paperclip and Totp columns not saving state * Work around Qt bug that causes isSectionHidden to return false after restoring state due to the section actually only being set to 0 width. * Fixes #5317 --- src/gui/entry/EntryView.cpp | 21 +++++++++++++++------ src/gui/entry/EntryView.h | 1 + 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/gui/entry/EntryView.cpp b/src/gui/entry/EntryView.cpp index 18a69687d..a387575e9 100644 --- a/src/gui/entry/EntryView.cpp +++ b/src/gui/entry/EntryView.cpp @@ -379,8 +379,7 @@ void EntryView::showHeaderMenu(const QPoint& position) continue; } int columnIndex = action->data().toInt(); - bool hidden = header()->isSectionHidden(columnIndex) || (header()->sectionSize(columnIndex) == 0); - action->setChecked(!hidden); + action->setChecked(!isColumnHidden(columnIndex)); } m_headerMenu->popup(mapToGlobal(position)); @@ -408,6 +407,7 @@ void EntryView::toggleColumnVisibility(QAction* action) if (header()->sectionSize(columnIndex) == 0) { header()->resizeSection(columnIndex, header()->defaultSectionSize()); } + resetFixedColumns(); return; } if ((header()->count() - header()->hiddenSectionCount()) > 1) { @@ -460,11 +460,15 @@ void EntryView::fitColumnsToContents() */ void EntryView::resetFixedColumns() { - header()->setSectionResizeMode(EntryModel::Paperclip, QHeaderView::Fixed); - header()->resizeSection(EntryModel::Paperclip, header()->minimumSectionSize()); + if (!isColumnHidden(EntryModel::Paperclip)) { + header()->setSectionResizeMode(EntryModel::Paperclip, QHeaderView::Fixed); + header()->resizeSection(EntryModel::Paperclip, header()->minimumSectionSize()); + } - header()->setSectionResizeMode(EntryModel::Totp, QHeaderView::Fixed); - header()->resizeSection(EntryModel::Totp, header()->minimumSectionSize()); + if (!isColumnHidden(EntryModel::Totp)) { + header()->setSectionResizeMode(EntryModel::Totp, QHeaderView::Fixed); + header()->resizeSection(EntryModel::Totp, header()->minimumSectionSize()); + } } /** @@ -533,3 +537,8 @@ void EntryView::showEvent(QShowEvent* event) m_columnsNeedRelayout = false; } } + +bool EntryView::isColumnHidden(int logicalIndex) +{ + return header()->isSectionHidden(logicalIndex) || header()->sectionSize(logicalIndex) == 0; +} diff --git a/src/gui/entry/EntryView.h b/src/gui/entry/EntryView.h index e32aa4729..65cbf104a 100644 --- a/src/gui/entry/EntryView.h +++ b/src/gui/entry/EntryView.h @@ -80,6 +80,7 @@ private slots: private: void resetFixedColumns(); + bool isColumnHidden(int logicalIndex); EntryModel* const m_model; SortFilterHideProxyModel* const m_sortModel; From 1c27dccabbd179777557f2bd481c1f7cb446b098 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Tue, 25 Aug 2020 18:30:52 -0400 Subject: [PATCH 05/15] Use selected settings in password generator on load * Fixes #5336 --- src/gui/PasswordGeneratorWidget.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gui/PasswordGeneratorWidget.cpp b/src/gui/PasswordGeneratorWidget.cpp index 78b65b400..07dda651b 100644 --- a/src/gui/PasswordGeneratorWidget.cpp +++ b/src/gui/PasswordGeneratorWidget.cpp @@ -170,6 +170,7 @@ void PasswordGeneratorWidget::loadSettings() // Set advanced mode m_ui->buttonAdvancedMode->setChecked(advanced); setAdvancedMode(advanced); + updateGenerator(); } void PasswordGeneratorWidget::saveSettings() From 9cf93111d65af68225c551b33a37a2516ce6b51b Mon Sep 17 00:00:00 2001 From: Bernhard Date: Tue, 1 Sep 2020 12:29:21 +0200 Subject: [PATCH 06/15] Fix heap-use-after-free & error-msg output in keepassxc-cli * Fixes #5367 --- src/cli/keepassxc-cli.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cli/keepassxc-cli.cpp b/src/cli/keepassxc-cli.cpp index a9b276fda..5d326f46e 100644 --- a/src/cli/keepassxc-cli.cpp +++ b/src/cli/keepassxc-cli.cpp @@ -158,7 +158,7 @@ void enterInteractiveMode(const QStringList& arguments) auto cmd = Commands::getCommand(args[0]); if (!cmd) { - err << QObject::tr("Unknown command %1").arg(args[0]) << "\n"; + err << QObject::tr("Unknown command %1").arg(args[0]) << endl; continue; } else if (cmd->name == "quit" || cmd->name == "exit") { break; @@ -167,6 +167,7 @@ void enterInteractiveMode(const QStringList& arguments) cmd->currentDatabase = currentDatabase; cmd->execute(args); currentDatabase = cmd->currentDatabase; + cmd->currentDatabase.reset(); } if (currentDatabase) { From c67ebf19d45c8371c93d86587608e4b4ff44cd4a Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Wed, 26 Aug 2020 11:01:40 -0400 Subject: [PATCH 07/15] Fix AutoOpen IfDevice matching, again * Fix case where only exclusions are entered (eg, !COMPUTER1, !COMPUTER2) which should allow opening the database on every other computer name. --- src/gui/DatabaseWidget.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index d5ac7eb3e..5cfe26e03 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -2061,7 +2061,7 @@ void DatabaseWidget::processAutoOpen() // negated using '!' auto ifDevice = entry->attribute("IfDevice"); if (!ifDevice.isEmpty()) { - bool loadDb = true; + bool loadDb = false; auto hostName = QHostInfo::localHostName(); for (auto& device : ifDevice.split(",")) { device = device.trimmed(); @@ -2070,12 +2070,13 @@ void DatabaseWidget::processAutoOpen() // Machine name matched an exclusion, don't load this database loadDb = false; break; + } else { + // Not matching an exclusion allows loading on all machines + loadDb = true; } } else if (device.compare(hostName, Qt::CaseInsensitive) == 0) { + // Explicitly named for loading loadDb = true; - } else { - // Don't load the database if there are devices not starting with '!' - loadDb = false; } } if (!loadDb) { From 9bab5d5a33cbf8bcb87ac1512f45d7829392c19a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sami=20V=C3=A4nttinen?= Date: Sat, 5 Sep 2020 16:00:36 +0300 Subject: [PATCH 08/15] Don't mark URL references as invalid URL (#5380) --- src/core/Tools.cpp | 3 ++- tests/TestBrowser.cpp | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/Tools.cpp b/src/core/Tools.cpp index d29e92bff..7e2b65bcd 100644 --- a/src/core/Tools.cpp +++ b/src/core/Tools.cpp @@ -263,7 +263,8 @@ namespace Tools bool checkUrlValid(const QString& urlField) { - if (urlField.isEmpty() || urlField.startsWith("cmd://", Qt::CaseInsensitive)) { + if (urlField.isEmpty() || urlField.startsWith("cmd://", Qt::CaseInsensitive) + || urlField.startsWith("{REF:A", Qt::CaseInsensitive)) { return true; } diff --git a/tests/TestBrowser.cpp b/tests/TestBrowser.cpp index 3e518c1e2..95189fec7 100644 --- a/tests/TestBrowser.cpp +++ b/tests/TestBrowser.cpp @@ -437,6 +437,7 @@ void TestBrowser::testValidURLs() urls["http:/example.com"] = false; urls["cmd://C:/Toolchains/msys2/usr/bin/mintty \"ssh jon@192.168.0.1:22\""] = true; urls["file:///Users/testUser/Code/test.html"] = true; + urls["{REF:A@I:46C9B1FFBD4ABC4BBB260C6190BAD20C} "] = true; QHashIterator i(urls); while (i.hasNext()) { From e391dd182deb9c80ac61800d4c0936c67adb2b71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sami=20V=C3=A4nttinen?= Date: Sun, 13 Sep 2020 17:38:19 +0300 Subject: [PATCH 09/15] Fix Best-Matching ..again (#5316) Co-authored-by: Jonathan White --- src/browser/BrowserAction.cpp | 8 +- src/browser/BrowserService.cpp | 281 ++++++++++++++++----------------- src/browser/BrowserService.h | 36 ++--- tests/TestBrowser.cpp | 259 +++++++++++++++++++----------- tests/TestBrowser.h | 2 + 5 files changed, 329 insertions(+), 257 deletions(-) diff --git a/src/browser/BrowserAction.cpp b/src/browser/BrowserAction.cpp index 8e0c26909..65fe4cb7b 100644 --- a/src/browser/BrowserAction.cpp +++ b/src/browser/BrowserAction.cpp @@ -267,8 +267,8 @@ QJsonObject BrowserAction::handleGetLogins(const QJsonObject& json, const QStrin return getErrorReply(action, ERROR_KEEPASS_CANNOT_DECRYPT_MESSAGE); } - const QString url = decrypted.value("url").toString(); - if (url.isEmpty()) { + const QString siteUrl = decrypted.value("url").toString(); + if (siteUrl.isEmpty()) { return getErrorReply(action, ERROR_KEEPASS_NO_URL_PROVIDED); } @@ -281,10 +281,10 @@ QJsonObject BrowserAction::handleGetLogins(const QJsonObject& json, const QStrin } const QString id = decrypted.value("id").toString(); - const QString submit = decrypted.value("submitUrl").toString(); + const QString formUrl = decrypted.value("submitUrl").toString(); const QString auth = decrypted.value("httpAuth").toString(); const bool httpAuth = auth.compare(TRUE_STR, Qt::CaseSensitive) == 0 ? true : false; - const QJsonArray users = browserService()->findMatchingEntries(id, url, submit, "", keyList, httpAuth); + const QJsonArray users = browserService()->findMatchingEntries(id, siteUrl, formUrl, "", keyList, httpAuth); if (users.isEmpty()) { return getErrorReply(action, ERROR_KEEPASS_NO_LOGINS_FOUND); diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp index eb752996c..08c9f88da 100644 --- a/src/browser/BrowserService.cpp +++ b/src/browser/BrowserService.cpp @@ -371,8 +371,8 @@ QString BrowserService::getKey(const QString& id) } QJsonArray BrowserService::findMatchingEntries(const QString& dbid, - const QString& url, - const QString& submitUrl, + const QString& siteUrlStr, + const QString& formUrlStr, const QString& realm, const StringPairList& keyList, const bool httpAuth) @@ -380,13 +380,13 @@ QJsonArray BrowserService::findMatchingEntries(const QString& dbid, Q_UNUSED(dbid); const bool alwaysAllowAccess = browserSettings()->alwaysAllowAccess(); const bool ignoreHttpAuth = browserSettings()->httpAuthPermission(); - const QString host = QUrl(url).host(); - const QString submitHost = QUrl(submitUrl).host(); + const QString siteHost = QUrl(siteUrlStr).host(); + const QString formHost = QUrl(formUrlStr).host(); // Check entries for authorization QList pwEntriesToConfirm; QList pwEntries; - for (auto* entry : searchEntries(url, submitUrl, keyList)) { + for (auto* entry : searchEntries(siteUrlStr, formUrlStr, keyList)) { if (entry->customData()->contains(BrowserService::OPTION_HIDE_ENTRY) && entry->customData()->value(BrowserService::OPTION_HIDE_ENTRY) == TRUE_STR) { continue; @@ -403,7 +403,7 @@ QJsonArray BrowserService::findMatchingEntries(const QString& dbid, continue; } - switch (checkAccess(entry, host, submitHost, realm)) { + switch (checkAccess(entry, siteHost, formHost, realm)) { case Denied: continue; @@ -422,7 +422,8 @@ QJsonArray BrowserService::findMatchingEntries(const QString& dbid, } // Confirm entries - QList selectedEntriesToConfirm = confirmEntries(pwEntriesToConfirm, url, host, submitHost, realm, httpAuth); + QList selectedEntriesToConfirm = + confirmEntries(pwEntriesToConfirm, siteUrlStr, siteHost, formHost, realm, httpAuth); if (!selectedEntriesToConfirm.isEmpty()) { pwEntries.append(selectedEntriesToConfirm); } @@ -437,7 +438,7 @@ QJsonArray BrowserService::findMatchingEntries(const QString& dbid, } // Sort results - pwEntries = sortEntries(pwEntries, host, submitUrl, url); + pwEntries = sortEntries(pwEntries, siteUrlStr, formUrlStr); // Fill the list QJsonArray result; @@ -451,8 +452,8 @@ QJsonArray BrowserService::findMatchingEntries(const QString& dbid, void BrowserService::addEntry(const QString& dbid, const QString& login, const QString& password, - const QString& url, - const QString& submitUrl, + const QString& siteUrlStr, + const QString& formUrlStr, const QString& realm, const QString& group, const QString& groupUuid, @@ -467,8 +468,8 @@ void BrowserService::addEntry(const QString& dbid, auto* entry = new Entry(); entry->setUuid(QUuid::createUuid()); - entry->setTitle(QUrl(url).host()); - entry->setUrl(url); + entry->setTitle(QUrl(siteUrlStr).host()); + entry->setUrl(siteUrlStr); entry->setIcon(KEEPASSXCBROWSER_DEFAULT_ICON); entry->setUsername(login); entry->setPassword(password); @@ -487,8 +488,8 @@ void BrowserService::addEntry(const QString& dbid, entry->setGroup(getDefaultEntryGroup(db)); } - const QString host = QUrl(url).host(); - const QString submitHost = QUrl(submitUrl).host(); + const QString host = QUrl(siteUrlStr).host(); + const QString submitHost = QUrl(formUrlStr).host(); BrowserEntryConfig config; config.allow(host); @@ -505,8 +506,8 @@ bool BrowserService::updateEntry(const QString& dbid, const QString& uuid, const QString& login, const QString& password, - const QString& url, - const QString& submitUrl) + const QString& siteUrlStr, + const QString& formUrlStr) { // TODO: select database based on this key id Q_UNUSED(dbid); @@ -518,7 +519,7 @@ bool BrowserService::updateEntry(const QString& dbid, Entry* entry = db->rootGroup()->findEntryByUuid(Tools::hexToUuid(uuid)); if (!entry) { // If entry is not found for update, add a new one to the selected database - addEntry(dbid, login, password, url, submitUrl, "", "", "", db); + addEntry(dbid, login, password, siteUrlStr, formUrlStr, "", "", "", db); return true; } @@ -547,7 +548,7 @@ bool BrowserService::updateEntry(const QString& dbid, dialogResult = MessageBox::question( nullptr, tr("KeePassXC: Update Entry"), - tr("Do you want to update the information in %1 - %2?").arg(QUrl(url).host(), username), + tr("Do you want to update the information in %1 - %2?").arg(QUrl(siteUrlStr).host(), username), MessageBox::Save | MessageBox::Cancel, MessageBox::Cancel, MessageBox::Raise); @@ -570,7 +571,7 @@ bool BrowserService::updateEntry(const QString& dbid, } QList -BrowserService::searchEntries(const QSharedPointer& db, const QString& url, const QString& submitUrl) +BrowserService::searchEntries(const QSharedPointer& db, const QString& siteUrlStr, const QString& formUrlStr) { QList entries; auto* rootGroup = db->rootGroup(); @@ -590,25 +591,29 @@ BrowserService::searchEntries(const QSharedPointer& db, const QString& // Search for additional URL's starting with KP2A_URL for (const auto& key : entry->attributes()->keys()) { - if (key.startsWith(ADDITIONAL_URL) && handleURL(entry->attributes()->value(key), url, submitUrl) + if (key.startsWith(ADDITIONAL_URL) && handleURL(entry->attributes()->value(key), siteUrlStr, formUrlStr) && !entries.contains(entry)) { entries.append(entry); continue; } } - if (!handleURL(entry->url(), url, submitUrl)) { + if (!handleURL(entry->url(), siteUrlStr, formUrlStr)) { continue; } - entries.append(entry); + // Additional URL check may have already inserted the entry to the list + if (!entries.contains(entry)) { + entries.append(entry); + } } } return entries; } -QList BrowserService::searchEntries(const QString& url, const QString& submitUrl, const StringPairList& keyList) +QList +BrowserService::searchEntries(const QString& siteUrlStr, const QString& formUrlStr, const StringPairList& keyList) { // Check if database is connected with KeePassXC-Browser auto databaseConnected = [&](const QSharedPointer& db) { @@ -638,11 +643,11 @@ QList BrowserService::searchEntries(const QString& url, const QString& s } // Search entries matching the hostname - QString hostname = QUrl(url).host(); + QString hostname = QUrl(siteUrlStr).host(); QList entries; do { for (const auto& db : databases) { - entries << searchEntries(db, url, submitUrl); + entries << searchEntries(db, siteUrlStr, formUrlStr); } } while (entries.isEmpty() && removeFirstDomain(hostname)); @@ -722,47 +727,30 @@ void BrowserService::convertAttributesToCustomData(QSharedPointer db) } } -QList BrowserService::sortEntries(QList& pwEntries, - const QString& host, - const QString& entryUrl, - const QString& fullUrl) +QList +BrowserService::sortEntries(QList& pwEntries, const QString& siteUrlStr, const QString& formUrlStr) { - QUrl url(entryUrl); - if (url.scheme().isEmpty()) { - url.setScheme("https"); - } - - const QString submitUrl = url.toString(QUrl::StripTrailingSlash); - const QString baseSubmitUrl = - url.toString(QUrl::StripTrailingSlash | QUrl::RemovePath | QUrl::RemoveQuery | QUrl::RemoveFragment); - // Build map of prioritized entries QMultiMap priorities; for (auto* entry : pwEntries) { - priorities.insert(sortPriority(entry, host, submitUrl, baseSubmitUrl, fullUrl), entry); + priorities.insert(sortPriority(getEntryURLs(entry), siteUrlStr, formUrlStr), entry); } + auto keys = priorities.uniqueKeys(); + std::sort(keys.begin(), keys.end(), [](int l, int r) { return l > r; }); + QList results; - QString field = browserSettings()->sortByTitle() ? "Title" : "UserName"; - for (int i = 100; i >= 0; i -= 5) { - if (priorities.count(i) > 0) { - // Sort same priority entries by Title or UserName - auto entries = priorities.values(i); - std::sort(entries.begin(), entries.end(), [&field](Entry* left, Entry* right) { - return (QString::localeAwareCompare(left->attributes()->value(field), right->attributes()->value(field)) - < 0) - || ((QString::localeAwareCompare(left->attributes()->value(field), - right->attributes()->value(field)) - == 0) - && (QString::localeAwareCompare(left->attributes()->value("UserName"), - right->attributes()->value("UserName")) - < 0)); - }); - results << entries; - if (browserSettings()->bestMatchOnly() && !pwEntries.isEmpty()) { - // Early out once we find the highest batch of matches - break; - } + auto sortField = browserSettings()->sortByTitle() ? EntryAttributes::TitleKey : EntryAttributes::UserNameKey; + for (auto key : keys) { + // Sort same priority entries by Title or UserName + auto entries = priorities.values(key); + std::sort(entries.begin(), entries.end(), [&sortField](Entry* left, Entry* right) { + return QString::localeAwareCompare(left->attribute(sortField), right->attribute(sortField)); + }); + results << entries; + if (browserSettings()->bestMatchOnly() && !results.isEmpty()) { + // Early out once we find the highest batch of matches + break; } } @@ -770,9 +758,9 @@ QList BrowserService::sortEntries(QList& pwEntries, } QList BrowserService::confirmEntries(QList& pwEntriesToConfirm, - const QString& url, - const QString& host, - const QString& submitHost, + const QString& siteUrlStr, + const QString& siteHost, + const QString& formUrlStr, const QString& realm, const bool httpAuth) { @@ -790,9 +778,9 @@ QList BrowserService::confirmEntries(QList& pwEntriesToConfirm, auto entry = pwEntriesToConfirm[item->row()]; BrowserEntryConfig config; config.load(entry); - config.deny(host); - if (!submitHost.isEmpty() && host != submitHost) { - config.deny(submitHost); + config.deny(siteHost); + if (!formUrlStr.isEmpty() && siteHost != formUrlStr) { + config.deny(formUrlStr); } if (!realm.isEmpty()) { config.setRealm(realm); @@ -800,7 +788,7 @@ QList BrowserService::confirmEntries(QList& pwEntriesToConfirm, config.save(entry); }); - accessControlDialog.setItems(pwEntriesToConfirm, url, httpAuth); + accessControlDialog.setItems(pwEntriesToConfirm, siteUrlStr, httpAuth); QList allowedEntries; if (accessControlDialog.exec() == QDialog::Accepted) { @@ -810,9 +798,9 @@ QList BrowserService::confirmEntries(QList& pwEntriesToConfirm, if (accessControlDialog.remember()) { BrowserEntryConfig config; config.load(entry); - config.allow(host); - if (!submitHost.isEmpty() && host != submitHost) { - config.allow(submitHost); + config.allow(siteHost); + if (!formUrlStr.isEmpty() && siteHost != formUrlStr) { + config.allow(formUrlStr); } if (!realm.isEmpty()) { config.setRealm(realm); @@ -871,7 +859,7 @@ QJsonObject BrowserService::prepareEntry(const Entry* entry) } BrowserService::Access -BrowserService::checkAccess(const Entry* entry, const QString& host, const QString& submitHost, const QString& realm) +BrowserService::checkAccess(const Entry* entry, const QString& siteHost, const QString& formHost, const QString& realm) { if (entry->isExpired()) { return browserSettings()->allowExpiredCredentials() ? Allowed : Denied; @@ -881,10 +869,10 @@ BrowserService::checkAccess(const Entry* entry, const QString& host, const QStri if (!config.load(entry)) { return Unknown; } - if ((config.isAllowed(host)) && (submitHost.isEmpty() || config.isAllowed(submitHost))) { + if ((config.isAllowed(siteHost)) && (formHost.isEmpty() || config.isAllowed(formHost))) { return Allowed; } - if ((config.isDenied(host)) || (!submitHost.isEmpty() && config.isDenied(submitHost))) { + if ((config.isDenied(siteHost)) || (!formHost.isEmpty() && config.isDenied(formHost))) { return Denied; } if (!realm.isEmpty() && config.realm() != realm) { @@ -919,66 +907,72 @@ Group* BrowserService::getDefaultEntryGroup(const QSharedPointer& sele return group; } -int BrowserService::sortPriority(const Entry* entry, - const QString& host, - const QString& submitUrl, - const QString& baseSubmitUrl, - const QString& fullUrl) const +// Returns the maximum sort priority given a set of match urls and the +// extension provided site and form url. +int BrowserService::sortPriority(const QStringList& urls, const QString& siteUrlStr, const QString& formUrlStr) { - QUrl url(entry->url()); - if (url.scheme().isEmpty()) { - url.setScheme("https"); - } + QList priorityList; + // NOTE: QUrl::matches is utterly broken in Qt < 5.11, so we work around that + // by removing parts of the url that we don't match and direct matching others + const auto stdOpts = QUrl::RemoveFragment | QUrl::RemoveUserInfo; + const auto siteUrl = QUrl(siteUrlStr).adjusted(stdOpts); + const auto formUrl = QUrl(formUrlStr).adjusted(stdOpts); - // Add the empty path to the URL if it's missing - if (url.path().isEmpty() && !url.hasFragment() && !url.hasQuery()) { - url.setPath("/"); - } + auto getPriority = [&](const QString& givenUrl) { + auto url = QUrl::fromUserInput(givenUrl).adjusted(stdOpts); - const QString entryURL = url.toString(QUrl::StripTrailingSlash); - const QString baseEntryURL = - url.toString(QUrl::StripTrailingSlash | QUrl::RemovePath | QUrl::RemoveQuery | QUrl::RemoveFragment); + // Default to https scheme if undefined + if (url.scheme().isEmpty() || !givenUrl.contains("://")) { + url.setScheme("https"); + } - if (!url.host().contains(".") && url.host() != "localhost") { + // Add the empty path to the URL if it's missing. + // URL's from the extension always have a path set, entry URL's can be without. + if (url.path().isEmpty() && !url.hasFragment() && !url.hasQuery()) { + url.setPath("/"); + } + + // Reject invalid urls and hosts, except 'localhost', and scheme mismatch + if (!url.isValid() || (!url.host().contains(".") && url.host() != "localhost") + || url.scheme() != siteUrl.scheme()) { + return 0; + } + + // Exact match with site url or form url + if (url.matches(siteUrl, QUrl::None) || url.matches(formUrl, QUrl::None)) { + return 100; + } + + // Exact match without the query string + if (url.matches(siteUrl, QUrl::RemoveQuery) || url.matches(formUrl, QUrl::RemoveQuery)) { + return 90; + } + + // Match without path (ie, FQDN match), form url prioritizes lower than site url + if (url.host() == siteUrl.host()) { + return 80; + } + if (url.host() == formUrl.host()) { + return 70; + } + + // Site/form url ends with given url (subdomain mismatch) + if (siteUrl.host().endsWith(url.host())) { + return 60; + } + if (formUrl.host().endsWith(url.host())) { + return 50; + } + + // No valid match found return 0; + }; + + for (const auto& entryUrl : urls) { + priorityList << getPriority(entryUrl); } - if (fullUrl == entryURL) { - return 100; - } - if (submitUrl == entryURL) { - return 95; - } - if (submitUrl.startsWith(entryURL) && entryURL != host && baseSubmitUrl != entryURL) { - return 90; - } - if (submitUrl.startsWith(baseEntryURL) && entryURL != host && baseSubmitUrl != baseEntryURL) { - return 80; - } - if (entryURL == host) { - return 70; - } - if (entryURL == baseSubmitUrl) { - return 60; - } - if (entryURL.startsWith(submitUrl)) { - return 50; - } - if (entryURL.startsWith(baseSubmitUrl) && baseSubmitUrl != host) { - return 40; - } - if (submitUrl.startsWith(entryURL)) { - return 30; - } - if (submitUrl.startsWith(baseEntryURL)) { - return 20; - } - if (entryURL.startsWith(host)) { - return 10; - } - if (host.startsWith(entryURL)) { - return 5; - } - return 0; + + return *std::max_element(priorityList.begin(), priorityList.end()); } bool BrowserService::schemeFound(const QString& url) @@ -1004,7 +998,7 @@ bool BrowserService::removeFirstDomain(QString& hostname) return false; } -bool BrowserService::handleURL(const QString& entryUrl, const QString& url, const QString& submitUrl) +bool BrowserService::handleURL(const QString& entryUrl, const QString& siteUrlStr, const QString& formUrlStr) { if (entryUrl.isEmpty()) { return false; @@ -1022,8 +1016,8 @@ bool BrowserService::handleURL(const QString& entryUrl, const QString& url, cons } // Make a direct compare if a local file is used - if (url.contains("file://")) { - return entryUrl == submitUrl; + if (siteUrlStr.contains("file://")) { + return entryUrl == formUrlStr; } // URL host validation fails @@ -1032,7 +1026,7 @@ bool BrowserService::handleURL(const QString& entryUrl, const QString& url, cons } // Match port, if used - QUrl siteQUrl(url); + QUrl siteQUrl(siteUrlStr); if (entryQUrl.port() > 0 && entryQUrl.port() != siteQUrl.port()) { return false; } @@ -1056,17 +1050,7 @@ bool BrowserService::handleURL(const QString& entryUrl, const QString& url, cons // Match the subdomains with the limited wildcard if (siteQUrl.host().endsWith(entryQUrl.host())) { - if (!browserSettings()->bestMatchOnly()) { - return true; - } - - // Match the exact subdomain and path, or start of the path when entry's path is longer than plain "/" - if (siteQUrl.host() == entryQUrl.host()) { - if (siteQUrl.path() == entryQUrl.path() - || (entryQUrl.path().size() > 1 && siteQUrl.path().startsWith(entryQUrl.path()))) { - return true; - } - } + return true; } return false; @@ -1212,6 +1196,21 @@ bool BrowserService::checkLegacySettings(QSharedPointer db) return dialogResult == MessageBox::Yes; } +QStringList BrowserService::getEntryURLs(const Entry* entry) +{ + QStringList urlList; + urlList << entry->url(); + + // Handle additional URL's + for (const auto& key : entry->attributes()->keys()) { + if (key.startsWith(ADDITIONAL_URL)) { + urlList << entry->attributes()->value(key); + } + } + + return urlList; +} + void BrowserService::hideWindow() const { if (m_prevWindowState == WindowState::Minimized) { diff --git a/src/browser/BrowserService.h b/src/browser/BrowserService.h index f52b502b3..f84bf2880 100644 --- a/src/browser/BrowserService.h +++ b/src/browser/BrowserService.h @@ -63,8 +63,8 @@ public: void addEntry(const QString& dbid, const QString& login, const QString& password, - const QString& url, - const QString& submitUrl, + const QString& siteUrlStr, + const QString& formUrlStr, const QString& realm, const QString& group, const QString& groupUuid, @@ -73,12 +73,12 @@ public: const QString& uuid, const QString& login, const QString& password, - const QString& url, - const QString& submitUrl); + const QString& siteUrlStr, + const QString& formUrlStr); QJsonArray findMatchingEntries(const QString& dbid, - const QString& url, - const QString& submitUrl, + const QString& siteUrlStr, + const QString& formUrlStr, const QString& realm, const StringPairList& keyList, const bool httpAuth = false); @@ -118,35 +118,31 @@ private: Hidden }; - QList searchEntries(const QSharedPointer& db, const QString& url, const QString& submitUrl); - QList searchEntries(const QString& url, const QString& submitUrl, const StringPairList& keyList); QList - sortEntries(QList& pwEntries, const QString& host, const QString& submitUrl, const QString& fullUrl); + searchEntries(const QSharedPointer& db, const QString& siteUrlStr, const QString& formUrlStr); + QList searchEntries(const QString& siteUrlStr, const QString& formUrlStr, const StringPairList& keyList); + QList sortEntries(QList& pwEntries, const QString& siteUrlStr, const QString& formUrlStr); QList confirmEntries(QList& pwEntriesToConfirm, - const QString& url, - const QString& host, - const QString& submitUrl, + const QString& siteUrlStr, + const QString& siteHost, + const QString& formUrlStr, const QString& realm, const bool httpAuth); QJsonObject prepareEntry(const Entry* entry); QJsonArray getChildrenFromGroup(Group* group); - Access checkAccess(const Entry* entry, const QString& host, const QString& submitHost, const QString& realm); + Access checkAccess(const Entry* entry, const QString& siteHost, const QString& formHost, const QString& realm); Group* getDefaultEntryGroup(const QSharedPointer& selectedDb = {}); - int sortPriority(const Entry* entry, - const QString& host, - const QString& submitUrl, - const QString& baseSubmitUrl, - const QString& fullUrl) const; + int sortPriority(const QStringList& urls, const QString& siteUrlStr, const QString& formUrlStr); bool schemeFound(const QString& url); bool removeFirstDomain(QString& hostname); - bool handleURL(const QString& entryUrl, const QString& url, const QString& submitUrl); + bool handleURL(const QString& entryUrl, const QString& siteUrlStr, const QString& formUrlStr); QString baseDomain(const QString& hostname) const; QSharedPointer getDatabase(); QSharedPointer selectedDatabase(); QString getDatabaseRootUuid(); QString getDatabaseRecycleBinUuid(); - bool checkLegacySettings(QSharedPointer db); + QStringList getEntryURLs(const Entry* entry); void hideWindow() const; void raiseWindow(const bool force = false); diff --git a/tests/TestBrowser.cpp b/tests/TestBrowser.cpp index 95189fec7..c3a90e37c 100644 --- a/tests/TestBrowser.cpp +++ b/tests/TestBrowser.cpp @@ -128,59 +128,52 @@ void TestBrowser::testBaseDomain() void TestBrowser::testSortPriority() { - QString host = "github.com"; - QString submitUrl = "https://github.com/session"; - QString baseSubmitUrl = "https://github.com"; - QString fullUrl = "https://github.com/login"; + QFETCH(QString, entryUrl); + QFETCH(QString, siteUrl); + QFETCH(QString, formUrl); + QFETCH(int, expectedScore); - QScopedPointer entry1(new Entry()); - QScopedPointer entry2(new Entry()); - QScopedPointer entry3(new Entry()); - QScopedPointer entry4(new Entry()); - QScopedPointer entry5(new Entry()); - QScopedPointer entry6(new Entry()); - QScopedPointer entry7(new Entry()); - QScopedPointer entry8(new Entry()); - QScopedPointer entry9(new Entry()); - QScopedPointer entry10(new Entry()); - QScopedPointer entry11(new Entry()); + QScopedPointer entry(new Entry()); + entry->setUrl(entryUrl); - entry1->setUrl("https://github.com/login"); - entry2->setUrl("https://github.com/login"); - entry3->setUrl("https://github.com/"); - entry4->setUrl("github.com/login"); - entry5->setUrl("http://github.com"); - entry6->setUrl("http://github.com/login"); - entry7->setUrl("github.com"); - entry8->setUrl("github.com/login"); - entry9->setUrl("https://github"); // Invalid URL - entry10->setUrl("github.com"); - entry11->setUrl("https://github.com/login"); // Exact match + QCOMPARE(m_browserService->sortPriority(m_browserService->getEntryURLs(entry.data()), siteUrl, formUrl), + expectedScore); +} - // The extension uses the submitUrl as default for comparison - auto res1 = m_browserService->sortPriority(entry1.data(), host, "https://github.com/login", baseSubmitUrl, fullUrl); - auto res2 = m_browserService->sortPriority(entry2.data(), host, submitUrl, baseSubmitUrl, baseSubmitUrl); - auto res3 = m_browserService->sortPriority(entry3.data(), host, submitUrl, baseSubmitUrl, fullUrl); - auto res4 = m_browserService->sortPriority(entry4.data(), host, submitUrl, baseSubmitUrl, fullUrl); - auto res5 = m_browserService->sortPriority(entry5.data(), host, submitUrl, baseSubmitUrl, fullUrl); - auto res6 = m_browserService->sortPriority(entry6.data(), host, submitUrl, baseSubmitUrl, fullUrl); - auto res7 = m_browserService->sortPriority(entry7.data(), host, submitUrl, baseSubmitUrl, fullUrl); - auto res8 = m_browserService->sortPriority(entry8.data(), host, submitUrl, baseSubmitUrl, fullUrl); - auto res9 = m_browserService->sortPriority(entry9.data(), host, submitUrl, baseSubmitUrl, fullUrl); - auto res10 = m_browserService->sortPriority(entry10.data(), host, submitUrl, baseSubmitUrl, fullUrl); - auto res11 = m_browserService->sortPriority(entry11.data(), host, submitUrl, baseSubmitUrl, fullUrl); +void TestBrowser::testSortPriority_data() +{ + const QString siteUrl = "https://github.com/login"; + const QString formUrl = "https://github.com/session"; - QCOMPARE(res1, 100); - QCOMPARE(res2, 40); - QCOMPARE(res3, 90); - QCOMPARE(res4, 0); - QCOMPARE(res5, 0); - QCOMPARE(res6, 0); - QCOMPARE(res7, 0); - QCOMPARE(res8, 0); - QCOMPARE(res9, 0); - QCOMPARE(res10, 0); - QCOMPARE(res11, 100); + QTest::addColumn("entryUrl"); + QTest::addColumn("siteUrl"); + QTest::addColumn("formUrl"); + QTest::addColumn("expectedScore"); + + QTest::newRow("Exact Match") << siteUrl << siteUrl << siteUrl << 100; + QTest::newRow("Exact Match (site)") << siteUrl << siteUrl << formUrl << 100; + QTest::newRow("Exact Match (form)") << siteUrl << "https://github.net" << siteUrl << 100; + QTest::newRow("Exact Match No Trailing Slash") << "https://github.com" + << "https://github.com/" << formUrl << 100; + QTest::newRow("Exact Match No Scheme") << "github.com/login" << siteUrl << formUrl << 100; + QTest::newRow("Exact Match with Query") << "https://github.com/login?test=test#fragment" + << "https://github.com/login?test=test" << formUrl << 100; + + QTest::newRow("Site Query Mismatch") << siteUrl << siteUrl + "?test=test" << formUrl << 90; + + QTest::newRow("Path Mismatch (site)") << "https://github.com/" << siteUrl << formUrl << 80; + QTest::newRow("Path Mismatch (site) No Scheme") << "github.com" << siteUrl << formUrl << 80; + QTest::newRow("Path Mismatch (form)") << "https://github.com/" + << "https://github.net" << formUrl << 70; + + QTest::newRow("Subdomain Mismatch (site)") << siteUrl << "https://sub.github.com/" + << "https://github.net/" << 60; + QTest::newRow("Subdomain Mismatch (form)") << siteUrl << "https://github.net/" + << "https://sub.github.com/" << 50; + + QTest::newRow("Scheme Mismatch") << "http://github.com" << siteUrl << formUrl << 0; + QTest::newRow("Scheme Mismatch w/path") << "http://github.com/login" << siteUrl << formUrl << 0; + QTest::newRow("Invalid URL") << "http://github" << siteUrl << formUrl << 0; } void TestBrowser::testSearchEntries() @@ -344,14 +337,14 @@ void TestBrowser::testSubdomainsAndPaths() createEntries(entryURLs, root); - result = m_browserService->searchEntries(db, "https://accounts.example.com", "https://accounts.example.com"); + 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"); + 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 @@ -381,33 +374,32 @@ void TestBrowser::testSortEntries() "http://github.com", "http://github.com/login", "github.com", - "github.com/login", + "github.com/login?test=test", "https://github", // Invalid URL "github.com"}; auto entries = createEntries(urls, root); browserSettings()->setBestMatchOnly(false); - auto result = m_browserService->sortEntries( - entries, "github.com", "https://github.com/session", "https://github.com"); // entries, host, submitUrl + browserSettings()->setSortByUsername(true); + auto result = m_browserService->sortEntries(entries, "https://github.com/login", "https://github.com/session"); QCOMPARE(result.size(), 10); - QCOMPARE(result[0]->username(), QString("User 2")); - QCOMPARE(result[0]->url(), QString("https://github.com/")); - QCOMPARE(result[1]->username(), QString("User 0")); - QCOMPARE(result[1]->url(), QString("https://github.com/login_page")); - QCOMPARE(result[2]->username(), QString("User 1")); - QCOMPARE(result[2]->url(), QString("https://github.com/login")); - QCOMPARE(result[3]->username(), QString("User 3")); - QCOMPARE(result[3]->url(), QString("github.com/login")); + QCOMPARE(result[0]->username(), QString("User 1")); + QCOMPARE(result[0]->url(), urls[1]); + QCOMPARE(result[1]->username(), QString("User 3")); + QCOMPARE(result[1]->url(), urls[3]); + QCOMPARE(result[2]->username(), QString("User 7")); + QCOMPARE(result[2]->url(), urls[7]); + QCOMPARE(result[3]->username(), QString("User 0")); + QCOMPARE(result[3]->url(), urls[0]); // Test with a perfect match. That should be first in the list. - result = m_browserService->sortEntries( - entries, "github.com", "https://github.com/session", "https://github.com/login_page"); + result = m_browserService->sortEntries(entries, "https://github.com/login_page", "https://github.com/session"); QCOMPARE(result.size(), 10); QCOMPARE(result[0]->username(), QString("User 0")); QCOMPARE(result[0]->url(), QString("https://github.com/login_page")); - QCOMPARE(result[1]->username(), QString("User 2")); - QCOMPARE(result[1]->url(), QString("https://github.com/")); + QCOMPARE(result[1]->username(), QString("User 1")); + QCOMPARE(result[1]->url(), QString("https://github.com/login")); } QList TestBrowser::createEntries(QStringList& urls, Group* root) const @@ -458,45 +450,128 @@ void TestBrowser::testBestMatchingCredentials() browserSettings()->setBestMatchOnly(true); - auto result = m_browserService->searchEntries(db, "https://github.com/loginpage", "https://github.com/loginpage"); - QCOMPARE(result.size(), 1); - QCOMPARE(result[0]->url(), QString("https://github.com/loginpage")); + QString siteUrl = "https://github.com/loginpage"; + auto result = m_browserService->searchEntries(db, siteUrl, siteUrl); + auto sorted = m_browserService->sortEntries(result, siteUrl, siteUrl); + QCOMPARE(sorted.size(), 1); + QCOMPARE(sorted[0]->url(), siteUrl); - result = m_browserService->searchEntries(db, "https://github.com/justsomepage", "https://github.com/justsomepage"); - QCOMPARE(result.size(), 1); - QCOMPARE(result[0]->url(), QString("https://github.com/justsomepage")); + siteUrl = "https://github.com/justsomepage"; + result = m_browserService->searchEntries(db, siteUrl, siteUrl); + sorted = m_browserService->sortEntries(result, siteUrl, siteUrl); + QCOMPARE(sorted.size(), 1); + QCOMPARE(sorted[0]->url(), siteUrl); - result = m_browserService->searchEntries(db, "https://github.com/", "https://github.com/"); - m_browserService->sortEntries(entries, "github.com", "https://github.com/", "https://github.com/"); - QCOMPARE(result.size(), 1); - QCOMPARE(result[0]->url(), QString("https://github.com/")); + siteUrl = "https://github.com/"; + result = m_browserService->searchEntries(db, siteUrl, siteUrl); + sorted = m_browserService->sortEntries(entries, siteUrl, siteUrl); + QCOMPARE(sorted.size(), 1); + QCOMPARE(sorted[0]->url(), siteUrl); + // Without best-matching the URL with the path should be returned first browserSettings()->setBestMatchOnly(false); - result = m_browserService->searchEntries(db, "https://github.com/loginpage", "https://github.com/loginpage"); - QCOMPARE(result.size(), 3); - QCOMPARE(result[0]->url(), QString("https://github.com/loginpage")); + siteUrl = "https://github.com/loginpage"; + result = m_browserService->searchEntries(db, siteUrl, siteUrl); + sorted = m_browserService->sortEntries(result, siteUrl, siteUrl); + QCOMPARE(sorted.size(), 3); + QCOMPARE(sorted[0]->url(), siteUrl); // Test with subdomains QStringList subdomainsUrls = {"https://sub.github.com/loginpage", "https://sub.github.com/justsomepage", - "https://bus.github.com/justsomepage"}; + "https://bus.github.com/justsomepage", + "https://subdomain.example.com/", + "https://subdomain.example.com", + "https://example.com"}; entries = createEntries(subdomainsUrls, root); browserSettings()->setBestMatchOnly(true); + siteUrl = "https://sub.github.com/justsomepage"; + result = m_browserService->searchEntries(db, siteUrl, siteUrl); + sorted = m_browserService->sortEntries(result, siteUrl, siteUrl); + QCOMPARE(sorted.size(), 1); + QCOMPARE(sorted[0]->url(), siteUrl); - result = m_browserService->searchEntries( - db, "https://sub.github.com/justsomepage", "https://sub.github.com/justsomepage"); - QCOMPARE(result.size(), 1); - QCOMPARE(result[0]->url(), QString("https://sub.github.com/justsomepage")); + siteUrl = "https://github.com/justsomepage"; + result = m_browserService->searchEntries(db, siteUrl, siteUrl); + sorted = m_browserService->sortEntries(result, siteUrl, siteUrl); + QCOMPARE(sorted.size(), 1); + QCOMPARE(sorted[0]->url(), siteUrl); - result = m_browserService->searchEntries(db, "https://github.com/justsomepage", "https://github.com/justsomepage"); - QCOMPARE(result.size(), 1); - QCOMPARE(result[0]->url(), QString("https://github.com/justsomepage")); + siteUrl = "https://sub.github.com/justsomepage?wehavesomeextra=here"; + result = m_browserService->searchEntries(db, siteUrl, siteUrl); + sorted = m_browserService->sortEntries(result, siteUrl, siteUrl); + QCOMPARE(sorted.size(), 1); + QCOMPARE(sorted[0]->url(), QString("https://sub.github.com/justsomepage")); - result = m_browserService->searchEntries(db, - "https://sub.github.com/justsomepage?wehavesomeextra=here", - "https://sub.github.com/justsomepage?wehavesomeextra=here"); - QCOMPARE(result.size(), 1); - QCOMPARE(result[0]->url(), QString("https://sub.github.com/justsomepage")); + // The matching should not care if there's a / path or not. + siteUrl = "https://subdomain.example.com/"; + result = m_browserService->searchEntries(db, siteUrl, siteUrl); + sorted = m_browserService->sortEntries(result, siteUrl, siteUrl); + QCOMPARE(sorted.size(), 2); + QCOMPARE(sorted[0]->url(), QString("https://subdomain.example.com/")); + QCOMPARE(sorted[1]->url(), QString("https://subdomain.example.com")); + + // Entries with https://example.com should be still returned even if the site URL has a subdomain. Those have the + // best match. + db = QSharedPointer::create(); + root = db->rootGroup(); + QStringList domainUrls = {"https://example.com", "https://example.com", "https://other.example.com"}; + entries = createEntries(domainUrls, root); + siteUrl = "https://subdomain.example.com"; + result = m_browserService->searchEntries(db, siteUrl, siteUrl); + sorted = m_browserService->sortEntries(result, siteUrl, siteUrl); + + QCOMPARE(sorted.size(), 2); + QCOMPARE(sorted[0]->url(), QString("https://example.com")); + QCOMPARE(sorted[1]->url(), QString("https://example.com")); + + // https://github.com/keepassxreboot/keepassxc/issues/4754 + db = QSharedPointer::create(); + root = db->rootGroup(); + QStringList fooUrls = {"https://example.com/foo", "https://example.com/bar"}; + entries = createEntries(fooUrls, root); + + for (const auto& url : fooUrls) { + result = m_browserService->searchEntries(db, url, url); + sorted = m_browserService->sortEntries(result, url, url); + QCOMPARE(sorted.size(), 1); + QCOMPARE(sorted[0]->url(), QString(url)); + } + + // https://github.com/keepassxreboot/keepassxc/issues/4734 + db = QSharedPointer::create(); + root = db->rootGroup(); + QStringList testUrls = {"http://some.domain.tld/somePath", "http://some.domain.tld/otherPath"}; + entries = createEntries(testUrls, root); + + for (const auto& url : testUrls) { + result = m_browserService->searchEntries(db, url, url); + sorted = m_browserService->sortEntries(result, url, url); + QCOMPARE(sorted.size(), 1); + QCOMPARE(sorted[0]->url(), QString(url)); + } +} + +void TestBrowser::testBestMatchingWithAdditionalURLs() +{ + auto db = QSharedPointer::create(); + auto* root = db->rootGroup(); + + QStringList urls = {"https://github.com/loginpage", "https://test.github.com/", "https://github.com/"}; + + auto entries = createEntries(urls, root); + browserSettings()->setBestMatchOnly(true); + + // Add an additional URL to the first entry + entries.first()->attributes()->set(BrowserService::ADDITIONAL_URL, "https://test.github.com/anotherpage"); + + // The first entry should be triggered + auto result = m_browserService->searchEntries( + db, "https://test.github.com/anotherpage", "https://test.github.com/anotherpage"); + auto sorted = m_browserService->sortEntries( + result, "https://test.github.com/anotherpage", "https://test.github.com/anotherpage"); + QCOMPARE(sorted.length(), 1); + QCOMPARE(sorted[0]->url(), urls[0]); } diff --git a/tests/TestBrowser.h b/tests/TestBrowser.h index c8be3d6ca..d6140e886 100644 --- a/tests/TestBrowser.h +++ b/tests/TestBrowser.h @@ -40,6 +40,7 @@ private slots: void testBaseDomain(); void testSortPriority(); + void testSortPriority_data(); void testSearchEntries(); void testSearchEntriesWithPort(); void testSearchEntriesWithAdditionalURLs(); @@ -48,6 +49,7 @@ private slots: void testSortEntries(); void testValidURLs(); void testBestMatchingCredentials(); + void testBestMatchingWithAdditionalURLs(); private: QList createEntries(QStringList& urls, Group* root) const; From 3c5bd0ff6b0ec58d094f6d0e61cf97db167b3219 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Mon, 14 Sep 2020 07:02:00 -0400 Subject: [PATCH 10/15] Fix sorting of reports * Fixes #4976 --- src/gui/reports/ReportsWidgetHealthcheck.cpp | 25 +++++++++++++++++++- src/gui/reports/ReportsWidgetHibp.cpp | 25 +++++++++++++++++++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/gui/reports/ReportsWidgetHealthcheck.cpp b/src/gui/reports/ReportsWidgetHealthcheck.cpp index 5f502b16b..bc42b1e01 100644 --- a/src/gui/reports/ReportsWidgetHealthcheck.cpp +++ b/src/gui/reports/ReportsWidgetHealthcheck.cpp @@ -76,6 +76,27 @@ namespace QList> m_items; bool m_anyKnownBad = false; }; + + class ReportSortProxyModel : public QSortFilterProxyModel + { + public: + ReportSortProxyModel(QObject* parent) + : QSortFilterProxyModel(parent){}; + ~ReportSortProxyModel() override = default; + + protected: + bool lessThan(const QModelIndex& left, const QModelIndex& right) const override + { + // Check if the display data is a number, convert and compare if so + bool ok = false; + int leftInt = sourceModel()->data(left).toString().toInt(&ok); + if (ok) { + return leftInt < sourceModel()->data(right).toString().toInt(); + } + // Otherwise use default sorting + return QSortFilterProxyModel::lessThan(left, right); + } + }; } // namespace Health::Health(QSharedPointer db) @@ -121,11 +142,12 @@ ReportsWidgetHealthcheck::ReportsWidgetHealthcheck(QWidget* parent) , m_ui(new Ui::ReportsWidgetHealthcheck()) , m_errorIcon(Resources::instance()->icon("dialog-error")) , m_referencesModel(new QStandardItemModel(this)) - , m_modelProxy(new QSortFilterProxyModel(this)) + , m_modelProxy(new ReportSortProxyModel(this)) { m_ui->setupUi(this); m_modelProxy->setSourceModel(m_referencesModel.data()); + m_modelProxy->setSortLocaleAware(true); m_ui->healthcheckTableView->setModel(m_modelProxy.data()); m_ui->healthcheckTableView->setSelectionMode(QAbstractItemView::NoSelection); m_ui->healthcheckTableView->horizontalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents); @@ -256,6 +278,7 @@ void ReportsWidgetHealthcheck::calculateHealth() } else { m_referencesModel->setHorizontalHeaderLabels(QStringList() << tr("") << tr("Title") << tr("Path") << tr("Score") << tr("Reason")); + m_ui->healthcheckTableView->sortByColumn(0, Qt::AscendingOrder); } m_ui->healthcheckTableView->resizeRowsToContents(); diff --git a/src/gui/reports/ReportsWidgetHibp.cpp b/src/gui/reports/ReportsWidgetHibp.cpp index 48e36518d..406c465b9 100644 --- a/src/gui/reports/ReportsWidgetHibp.cpp +++ b/src/gui/reports/ReportsWidgetHibp.cpp @@ -45,17 +45,38 @@ namespace return entry->customData()->contains(PasswordHealth::OPTION_KNOWN_BAD) && entry->customData()->value(PasswordHealth::OPTION_KNOWN_BAD) == TRUE_STR; } + + class ReportSortProxyModel : public QSortFilterProxyModel + { + public: + ReportSortProxyModel(QObject* parent) + : QSortFilterProxyModel(parent){}; + ~ReportSortProxyModel() override = default; + + protected: + bool lessThan(const QModelIndex& left, const QModelIndex& right) const override + { + // Sort count column by user data + if (left.column() == 2) { + return sourceModel()->data(left, Qt::UserRole).toInt() + < sourceModel()->data(right, Qt::UserRole).toInt(); + } + // Otherwise use default sorting + return QSortFilterProxyModel::lessThan(left, right); + } + }; } // namespace ReportsWidgetHibp::ReportsWidgetHibp(QWidget* parent) : QWidget(parent) , m_ui(new Ui::ReportsWidgetHibp()) , m_referencesModel(new QStandardItemModel(this)) - , m_modelProxy(new QSortFilterProxyModel(this)) + , m_modelProxy(new ReportSortProxyModel(this)) { m_ui->setupUi(this); m_modelProxy->setSourceModel(m_referencesModel.data()); + m_modelProxy->setSortLocaleAware(true); m_ui->hibpTableView->setModel(m_modelProxy.data()); m_ui->hibpTableView->setSelectionMode(QAbstractItemView::NoSelection); m_ui->hibpTableView->horizontalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents); @@ -167,6 +188,7 @@ void ReportsWidgetHibp::makeHibpTable() } row[2]->setForeground(red); + row[2]->setData(count, Qt::UserRole); m_referencesModel->appendRow(row); // Store entry pointer per table row (used in double click handler) @@ -198,6 +220,7 @@ void ReportsWidgetHibp::makeHibpTable() } m_ui->hibpTableView->resizeRowsToContents(); + m_ui->hibpTableView->sortByColumn(2, Qt::DescendingOrder); m_ui->stackedWidget->setCurrentIndex(1); } From 55e488905313de3b861ad1c3005337f3f072aee4 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sun, 13 Sep 2020 10:31:18 -0400 Subject: [PATCH 11/15] Fix layout and alignment of Entry and Group edit views * Fixes #5321 - Text alignment in the general tab of the entry and group edit views is fixed * Fixes #5300 - Errant scrollbar in the general tab is fixed * Fixes #4852 - Tabbing into notes field works as expected. To tab out, currently only Shift+Tab works. --- src/gui/EditWidget.cpp | 18 +- src/gui/entry/EditEntryWidget.cpp | 5 +- src/gui/entry/EditEntryWidget.h | 3 +- src/gui/entry/EditEntryWidgetMain.ui | 584 ++++++++++++++------------- src/gui/group/EditGroupWidget.cpp | 2 +- src/gui/group/EditGroupWidget.h | 3 +- src/gui/group/EditGroupWidgetMain.ui | 432 +++++++++++--------- src/gui/styles/base/basestyle.qss | 5 + 8 files changed, 562 insertions(+), 490 deletions(-) diff --git a/src/gui/EditWidget.cpp b/src/gui/EditWidget.cpp index 68a8d7d4a..72742b5ac 100644 --- a/src/gui/EditWidget.cpp +++ b/src/gui/EditWidget.cpp @@ -59,12 +59,18 @@ void EditWidget::addPage(const QString& labelText, const QIcon& icon, QWidget* w * from automatic resizing and it now should be able to fit into a user's monitor even if the monitor is only 768 * pixels high. */ - auto* scrollArea = new QScrollArea(m_ui->stackedWidget); - scrollArea->setFrameShape(QFrame::NoFrame); - scrollArea->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); - scrollArea->setWidget(widget); - scrollArea->setWidgetResizable(true); - m_ui->stackedWidget->addWidget(scrollArea); + if (widget->inherits("QScrollArea")) { + m_ui->stackedWidget->addWidget(widget); + } else { + auto* scrollArea = new QScrollArea(m_ui->stackedWidget); + scrollArea->setFrameShape(QFrame::NoFrame); + scrollArea->setFrameShadow(QFrame::Plain); + scrollArea->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + scrollArea->setSizeAdjustPolicy(QScrollArea::AdjustToContents); + scrollArea->setWidgetResizable(true); + scrollArea->setWidget(widget); + m_ui->stackedWidget->addWidget(scrollArea); + } m_ui->categoryList->addCategory(labelText, icon); } diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index 53aa04efa..d7d8b7b32 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -76,7 +76,7 @@ EditEntryWidget::EditEntryWidget(QWidget* parent) , m_historyUi(new Ui::EditEntryWidgetHistory()) , m_browserUi(new Ui::EditEntryWidgetBrowser()) , m_customData(new CustomData()) - , m_mainWidget(new QWidget()) + , m_mainWidget(new QScrollArea()) , m_advancedWidget(new QWidget()) , m_iconsWidget(new EditWidgetIcons()) , m_autoTypeWidget(new QWidget()) @@ -178,6 +178,9 @@ void EditEntryWidget::setupMain() m_mainUi->expirePresets->setMenu(createPresetsMenu()); connect(m_mainUi->expirePresets->menu(), SIGNAL(triggered(QAction*)), this, SLOT(useExpiryPreset(QAction*))); + + // HACK: Align username text with other line edits. Qt does not let you do this with an application stylesheet. + m_mainUi->usernameComboBox->lineEdit()->setStyleSheet("padding-left: 8px;"); } void EditEntryWidget::setupAdvanced() diff --git a/src/gui/entry/EditEntryWidget.h b/src/gui/entry/EditEntryWidget.h index 3d1835396..e359d1029 100644 --- a/src/gui/entry/EditEntryWidget.h +++ b/src/gui/entry/EditEntryWidget.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include "config-keepassx.h" @@ -174,7 +175,7 @@ private: const QScopedPointer m_browserUi; const QScopedPointer m_customData; - QWidget* const m_mainWidget; + QScrollArea* const m_mainWidget; QWidget* const m_advancedWidget; EditWidgetIcons* const m_iconsWidget; QWidget* const m_autoTypeWidget; diff --git a/src/gui/entry/EditEntryWidgetMain.ui b/src/gui/entry/EditEntryWidgetMain.ui index f96481a3f..183ca0388 100644 --- a/src/gui/entry/EditEntryWidgetMain.ui +++ b/src/gui/entry/EditEntryWidgetMain.ui @@ -1,278 +1,306 @@ - - - EditEntryWidgetMain - - - - 0 - 0 - 496 - 420 - - - - - 0 - - - 0 - - - 0 - - - 0 - - - 10 - - - 8 - - - - - - - - 0 - 1 - - - - - 0 - 100 - - - - Notes field - - - - - - - true - - - Toggle the checkbox to reveal the notes section. - - - Qt::AlignTop - - - - - - - - - Username field - - - - - - - - - Toggle notes visible - - - Toggle notes visible - - - Notes: - - - - - - - Qt::Vertical - - - - 20 - 40 - - - - - - - - - - 8 - - - - - false - - - Expiration field - - - true - - - - - - - - 0 - 0 - - - - Expiration Presets - - - Expiration presets - - - Presets - - - - - - - - - Password: - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - - - - URL: - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - - - - 8 - - - - - Url field - - - https://example.com - - - - - - - Download favicon for URL - - - Download favicon for URL - - - - - - - - - Title: - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - - - - Title field - - - - - - - Username: - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - - - - Password field - - - QLineEdit::Password - - - - - - - 0 - - - - - Toggle expiration - - - Toggle expiration - - - Expires: - - - - - - - - - - PasswordEdit - QLineEdit -
gui/PasswordEdit.h
- 1 - - - URLEdit - QLineEdit -
gui/URLEdit.h
- 1 - - - - titleEdit - usernameComboBox - passwordEdit - urlEdit - fetchFaviconButton - expireCheck - expireDatePicker - expirePresets - notesEnabled - notesEdit - - - - + + + EditEntryWidgetMain + + + + 0 + 0 + 539 + 523 + + + + Edit Entry + + + QFrame::NoFrame + + + QFrame::Plain + + + Qt::ScrollBarAlwaysOff + + + QAbstractScrollArea::AdjustToContents + + + true + + + + + 0 + 0 + 539 + 523 + + + + + 0 + + + 0 + + + 0 + + + 0 + + + 10 + + + 8 + + + + + + + + 0 + 1 + + + + + 0 + 100 + + + + Notes field + + + + + + + true + + + Toggle the checkbox to reveal the notes section. + + + Qt::AlignTop + + + + + + + + + Username field + + + + + + + + + Toggle notes visible + + + Toggle notes visible + + + Notes: + + + + + + + Qt::Vertical + + + + 20 + 40 + + + + + + + + + + 8 + + + + + false + + + Expiration field + + + true + + + + + + + + 0 + 0 + + + + Expiration Presets + + + Expiration presets + + + Presets + + + + + + + + + Password: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + URL: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + 8 + + + + + Url field + + + https://example.com + + + + + + + Download favicon for URL + + + Download favicon for URL + + + + + + + + + Title: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + Title field + + + + + + + Username: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + Password field + + + QLineEdit::Password + + + + + + + 0 + + + + + Toggle expiration + + + Toggle expiration + + + Expires: + + + + + + + + + + + PasswordEdit + QLineEdit +
gui/PasswordEdit.h
+ 1 + + + URLEdit + QLineEdit +
gui/URLEdit.h
+ 1 + + + + titleEdit + usernameComboBox + passwordEdit + urlEdit + fetchFaviconButton + expireCheck + expireDatePicker + expirePresets + notesEnabled + notesEdit + + + + diff --git a/src/gui/group/EditGroupWidget.cpp b/src/gui/group/EditGroupWidget.cpp index b77e49864..ba79cce18 100644 --- a/src/gui/group/EditGroupWidget.cpp +++ b/src/gui/group/EditGroupWidget.cpp @@ -62,7 +62,7 @@ private: EditGroupWidget::EditGroupWidget(QWidget* parent) : EditWidget(parent) , m_mainUi(new Ui::EditGroupWidgetMain()) - , m_editGroupWidgetMain(new QWidget()) + , m_editGroupWidgetMain(new QScrollArea()) , m_editGroupWidgetIcons(new EditWidgetIcons()) , m_editWidgetProperties(new EditWidgetProperties()) , m_group(nullptr) diff --git a/src/gui/group/EditGroupWidget.h b/src/gui/group/EditGroupWidget.h index cc8738d8c..ed1bb0179 100644 --- a/src/gui/group/EditGroupWidget.h +++ b/src/gui/group/EditGroupWidget.h @@ -20,6 +20,7 @@ #include #include +#include #include "core/Group.h" #include "gui/EditWidget.h" @@ -78,7 +79,7 @@ private: const QScopedPointer m_mainUi; - QPointer m_editGroupWidgetMain; + QPointer m_editGroupWidgetMain; QPointer m_editGroupWidgetIcons; QPointer m_editWidgetProperties; diff --git a/src/gui/group/EditGroupWidgetMain.ui b/src/gui/group/EditGroupWidgetMain.ui index 9531cc847..faa8a30ff 100644 --- a/src/gui/group/EditGroupWidgetMain.ui +++ b/src/gui/group/EditGroupWidgetMain.ui @@ -1,215 +1,243 @@ EditGroupWidgetMain - + 0 0 - 410 - 430 + 539 + 523 - - - 0 + + Edit Group + + + QFrame::NoFrame + + + QFrame::Plain + + + Qt::ScrollBarAlwaysOff + + + QAbstractScrollArea::AdjustToContents + + + true + + + + + 0 + 0 + 539 + 523 + - - 0 - - - 0 - - - 0 - - - 10 - - - 8 - - - - - Toggle expiration - - - Expires: - - - - - - - Name field - - - - - - - false - - - Expiration field - - - true - - - - - - - Use default Auto-Type sequence of parent group - - - - - - - Auto-Type: - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - - - - Search: - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - - - - Auto-Type toggle for this and sub groups - - - - - - - - - Notes: - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - - - - Qt::Vertical - - - - 20 - 0 - - - - - - - - - - - - Qt::Horizontal - - - QSizePolicy::Fixed - - - - 30 - 0 - - - - - - - - false - - - Default auto-type sequence field - - - - - - - - - - - - - 0 - 0 - - - - - 16777215 - 120 - - - - Notes field - - - - - - - Name: - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - - - - Set default Auto-Type sequence - - - - - - - Search toggle for this and sub groups - - - - - - - Qt::Vertical - - - - 20 - 40 - - - - - + + + 0 + + + 0 + + + 0 + + + 0 + + + 10 + + + 8 + + + + + Toggle expiration + + + Expires: + + + + + + + Name field + + + + + + + false + + + Expiration field + + + true + + + + + + + Use default Auto-Type sequence of parent group + + + + + + + Auto-Type: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + Search: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + Auto-Type toggle for this and sub groups + + + + + + + + + Notes: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + Qt::Vertical + + + + 20 + 0 + + + + + + + + + + + + Qt::Horizontal + + + QSizePolicy::Fixed + + + + 30 + 0 + + + + + + + + false + + + Default auto-type sequence field + + + + + + + + + + + + + 0 + 0 + + + + + 16777215 + 120 + + + + Notes field + + + + + + + Name: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + Set default Auto-Type sequence + + + + + + + Search toggle for this and sub groups + + + + + + + Qt::Vertical + + + + 20 + 40 + + + + + + editName diff --git a/src/gui/styles/base/basestyle.qss b/src/gui/styles/base/basestyle.qss index 012c7cd0e..ff5d915bb 100644 --- a/src/gui/styles/base/basestyle.qss +++ b/src/gui/styles/base/basestyle.qss @@ -64,3 +64,8 @@ DatabaseWidget #SearchBanner, DatabaseWidget #KeeShareBanner { border: 1px solid palette(dark); padding: 2px; } + +QPlainTextEdit, QTextEdit { + background-color: palette(base); + padding-left: 4px; +} From 122051c91c2bb7f159c0cedb58df2119cb727836 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Wed, 16 Sep 2020 08:03:10 -0400 Subject: [PATCH 12/15] Restore natural sort on application load * Fixes #5435 --- src/gui/entry/EntryView.cpp | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/gui/entry/EntryView.cpp b/src/gui/entry/EntryView.cpp index a387575e9..53f03b989 100644 --- a/src/gui/entry/EntryView.cpp +++ b/src/gui/entry/EntryView.cpp @@ -109,21 +109,12 @@ EntryView::EntryView(QWidget* parent) header()->setContextMenuPolicy(Qt::CustomContextMenu); connect(header(), SIGNAL(customContextMenuRequested(QPoint)), SLOT(showHeaderMenu(QPoint))); - // clang-format off - connect(header(), SIGNAL(sectionCountChanged(int,int)), SIGNAL(viewStateChanged())); - // clang-format on + connect(header(), SIGNAL(sectionCountChanged(int, int)), SIGNAL(viewStateChanged())); + connect(header(), SIGNAL(sectionMoved(int, int, int)), SIGNAL(viewStateChanged())); + connect(header(), SIGNAL(sectionResized(int, int, int)), SIGNAL(viewStateChanged())); + connect(header(), SIGNAL(sortIndicatorChanged(int, Qt::SortOrder)), SLOT(sortIndicatorChanged(int, Qt::SortOrder))); // clang-format off - connect(header(), SIGNAL(sectionMoved(int,int,int)), SIGNAL(viewStateChanged())); - // clang-format on - - // clang-format off - connect(header(), SIGNAL(sectionResized(int,int,int)), SIGNAL(viewStateChanged())); - // clang-format on - - // clang-format off - connect(header(), SIGNAL(sortIndicatorChanged(int,Qt::SortOrder)), SLOT(sortIndicatorChanged(int,Qt::SortOrder))); - // clang-format on } void EntryView::contextMenuShortcutPressed() @@ -358,6 +349,8 @@ QByteArray EntryView::viewState() const */ bool EntryView::setViewState(const QByteArray& state) { + // Reset to unsorted first (https://bugreports.qt.io/browse/QTBUG-86694) + header()->setSortIndicator(-1, Qt::AscendingOrder); bool status = header()->restoreState(state); resetFixedColumns(); m_columnsNeedRelayout = state.isEmpty(); From 9886b1075fbddca0d4ef564c1bb481afcc199c3f Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sun, 20 Sep 2020 09:39:59 -0400 Subject: [PATCH 13/15] Cleanup config initialization, add local config options * Fix #5313, allow specifying local config path using environment variable and command line flag * Add command line flag `--localconfig ` to specify a file path to use for the local configuration settings. * Add environment variable support to set config files paths: `KPXC_CONFIG` and `KPXC_CONFIG_LOCAL` to override default locations. * Reorder startup sequence to load specified config files earlier to allow for theme settings and other early options to be picked up. * Removed old command line option `--pw`, no longer used. * Attempt a fix of application not closing when last window is gone. Only set `QApplication::setQuitOnLastWindowClosed(true)` when tray icon is enabled instead of always. --- docs/styles/dark.css | 1 + docs/topics/DownloadInstall.adoc | 2 + docs/topics/UserInterface.adoc | 37 +++++++++++++ src/core/Config.cpp | 90 ++++++++++++++++++-------------- src/core/Config.h | 7 +-- src/gui/MainWindow.cpp | 4 ++ src/main.cpp | 53 +++++++++---------- 7 files changed, 125 insertions(+), 69 deletions(-) diff --git a/docs/styles/dark.css b/docs/styles/dark.css index a32c366f3..8f7bd67b6 100644 --- a/docs/styles/dark.css +++ b/docs/styles/dark.css @@ -455,6 +455,7 @@ p{font-family: "Noto Sans",sans-serif !important} blockquote{color:var(--quotecolor) !important} .quoteblock{color:var(--textcolor)} code{color:var(--textcoloralt);background-color: var(--sidebarbackground) !important} +pre,pre>code{line-height:1.25; color:var(--textcoloralt);} .keyseq{color:var(--textcoloralt);} diff --git a/docs/topics/DownloadInstall.adoc b/docs/topics/DownloadInstall.adoc index 4e17c66bc..cc6f1fcb7 100644 --- a/docs/topics/DownloadInstall.adoc +++ b/docs/topics/DownloadInstall.adoc @@ -51,6 +51,8 @@ image::linux_store.png[] The Snap and Flatpak options are sandboxed applications (more secure). The Native option is installed with the operating system files. Read more about the limitations of these options here: https://keepassxc.org/docs/#faq-appsnap-yubikey[KeePassXC Snap FAQ] +NOTE: KeePassXC stores a configuration file in `~/.cache` to remember window position, recent files, and other local settings. If you mount this folder to a tmpdisk you will lose settings after reboot. + === macOS To install the KeePassXC app on macOS, double click on the downloaded DMG file and use the click and drag option as shown: diff --git a/docs/topics/UserInterface.adoc b/docs/topics/UserInterface.adoc index 1fee94608..1f0ca9cf2 100644 --- a/docs/topics/UserInterface.adoc +++ b/docs/topics/UserInterface.adoc @@ -48,4 +48,41 @@ image::compact_mode_comparison.png[] === Keyboard Shortcuts include::KeyboardShortcuts.adoc[tag=content, leveloffset=+1] + +// tag::advanced[] +=== Command-Line Options +You can use the following command line options to tailor the application to your preferences: + +---- +Usage: keepassxc.exe [options] [filename(s)] +KeePassXC - cross-platform password manager + +Options: + -?, -h, --help Displays help on commandline options. + --help-all Displays help including Qt specific options. + -v, --version Displays version information. + --config path to a custom config file + --localconfig path to a custom local config file + --keyfile key file of the database + --pw-stdin read password of the database from stdin + --debug-info Displays debugging information. + +Arguments: + filename(s) filenames of the password databases to open (*.kdbx) +---- + +Additionally, the following environment variables may be useful when running the application: + +[grid=rows, frame=none, width=75%] +|=== +|Env Var | Description + +|KPXC_CONFIG | Override default path to roaming configuration file +|KPXC_CONFIG_LOCAL | Override default path to local configuration file +|SSH_AUTH_SOCKET | Path of the unix file socket that the agent uses for communication with other processes (SSH Agent) +|QT_SCALE_FACTOR [numeric] | Defines a global scale factor for the whole application, including point-sized fonts. +|QT_SCREEN_SCALE_FACTORS [list] | Specifies scale factors for each screen. See https://doc.qt.io/qt-5/highdpi.html#high-dpi-support-in-qt +|QT_SCALE_FACTOR_ROUNDING_POLICY | Control device pixel ratio rounding to the nearest integer. See https://doc.qt.io/qt-5/highdpi.html#high-dpi-support-in-qt +|=== +// end::advanced[] // end::content[] diff --git a/src/core/Config.cpp b/src/core/Config.cpp index afb71f534..58ddd9ae4 100644 --- a/src/core/Config.cpp +++ b/src/core/Config.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -418,49 +419,17 @@ void Config::migrate() sync(); } -Config::Config(const QString& fileName, QObject* parent) +Config::Config(const QString& configFileName, const QString& localConfigFileName, QObject* parent) : QObject(parent) { - init(fileName); + init(configFileName, localConfigFileName); } Config::Config(QObject* parent) : QObject(parent) { - // Check if we are running in portable mode, if so store the config files local to the app - auto portablePath = QCoreApplication::applicationDirPath().append("/%1"); - if (QFile::exists(portablePath.arg(".portable"))) { - init(portablePath.arg("config/keepassxc.ini"), portablePath.arg("config/keepassxc_local.ini")); - return; - } - - QString configPath; - QString localConfigPath; - -#if defined(Q_OS_WIN) - configPath = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation); - localConfigPath = QStandardPaths::writableLocation(QStandardPaths::AppLocalDataLocation); -#elif defined(Q_OS_MACOS) - configPath = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation); - localConfigPath = QStandardPaths::writableLocation(QStandardPaths::CacheLocation); -#else - // On case-sensitive Operating Systems, force use of lowercase app directories - configPath = QStandardPaths::writableLocation(QStandardPaths::GenericConfigLocation) + "/keepassxc"; - localConfigPath = QStandardPaths::writableLocation(QStandardPaths::GenericCacheLocation) + "/keepassxc"; -#endif - - configPath += "/keepassxc"; - localConfigPath += "/keepassxc"; - -#ifdef QT_DEBUG - configPath += "_debug"; - localConfigPath += "_debug"; -#endif - - configPath += ".ini"; - localConfigPath += ".ini"; - - init(QDir::toNativeSeparators(configPath), QDir::toNativeSeparators(localConfigPath)); + auto configFiles = defaultConfigFiles(); + init(configFiles.first, configFiles.second); } Config::~Config() @@ -488,6 +457,45 @@ void Config::init(const QString& configFileName, const QString& localConfigFileN connect(qApp, &QCoreApplication::aboutToQuit, this, &Config::sync); } +QPair Config::defaultConfigFiles() +{ + // Check if we are running in portable mode, if so store the config files local to the app + auto portablePath = QCoreApplication::applicationDirPath().append("/%1"); + if (QFile::exists(portablePath.arg(".portable"))) { + return {portablePath.arg("config/keepassxc.ini"), portablePath.arg("config/keepassxc_local.ini")}; + } + + QString configPath; + QString localConfigPath; + +#if defined(Q_OS_WIN) + configPath = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation); + localConfigPath = QStandardPaths::writableLocation(QStandardPaths::AppLocalDataLocation); +#elif defined(Q_OS_MACOS) + configPath = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation); + localConfigPath = QStandardPaths::writableLocation(QStandardPaths::CacheLocation); +#else + // On case-sensitive Operating Systems, force use of lowercase app directories + configPath = QStandardPaths::writableLocation(QStandardPaths::GenericConfigLocation) + "/keepassxc"; + localConfigPath = QStandardPaths::writableLocation(QStandardPaths::GenericCacheLocation) + "/keepassxc"; +#endif + + QString suffix; +#ifdef QT_DEBUG + suffix = "_debug"; +#endif + + configPath += QString("/keepassxc%1.ini").arg(suffix); + localConfigPath += QString("/keepassxc%1.ini").arg(suffix); + + // Allow overriding the default location with env vars + const auto& env = QProcessEnvironment::systemEnvironment(); + configPath = env.value("KPXC_CONFIG", configPath); + localConfigPath = env.value("KPXC_CONFIG_LOCAL", localConfigPath); + + return {QDir::toNativeSeparators(configPath), QDir::toNativeSeparators(localConfigPath)}; +} + Config* Config::instance() { if (!m_instance) { @@ -497,12 +505,16 @@ Config* Config::instance() return m_instance; } -void Config::createConfigFromFile(const QString& file) +void Config::createConfigFromFile(const QString& configFileName, const QString& localConfigFileName) { if (m_instance) { delete m_instance; } - m_instance = new Config(file, qApp); + + auto defaultFiles = defaultConfigFiles(); + m_instance = new Config(configFileName.isEmpty() ? defaultFiles.first : configFileName, + localConfigFileName.isEmpty() ? defaultFiles.second : localConfigFileName, + qApp); } void Config::createTempFileInstance() @@ -514,7 +526,7 @@ void Config::createTempFileInstance() bool openResult = tmpFile->open(); Q_ASSERT(openResult); Q_UNUSED(openResult); - m_instance = new Config(tmpFile->fileName(), qApp); + m_instance = new Config(tmpFile->fileName(), "", qApp); tmpFile->setParent(m_instance); } diff --git a/src/core/Config.h b/src/core/Config.h index 64e8c945a..2ed4c6ec1 100644 --- a/src/core/Config.h +++ b/src/core/Config.h @@ -198,17 +198,18 @@ public: void resetToDefaults(); static Config* instance(); - static void createConfigFromFile(const QString& file); + static void createConfigFromFile(const QString& configFileName, const QString& localConfigFileName = {}); static void createTempFileInstance(); signals: void changed(ConfigKey key); private: - Config(const QString& fileName, QObject* parent = nullptr); + Config(const QString& configFileName, const QString& localConfigFileName, QObject* parent); explicit Config(QObject* parent); - void init(const QString& configFileName, const QString& localConfigFileName = ""); + void init(const QString& configFileName, const QString& localConfigFileName); void migrate(); + static QPair defaultConfigFiles(); static QPointer m_instance; diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index 81bbf3a08..f5e0541b3 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -1269,6 +1269,8 @@ bool MainWindow::saveLastDatabases() void MainWindow::updateTrayIcon() { if (isTrayIconEnabled()) { + QApplication::setQuitOnLastWindowClosed(false); + if (!m_trayIcon) { m_trayIcon = new QSystemTrayIcon(this); auto* menu = new QMenu(this); @@ -1307,6 +1309,8 @@ void MainWindow::updateTrayIcon() m_trayIcon->setIcon(resources()->trayIconLocked()); } } else { + QApplication::setQuitOnLastWindowClosed(true); + if (m_trayIcon) { m_trayIcon->hide(); delete m_trayIcon; diff --git a/src/main.cpp b/src/main.cpp index 7e340da4d..b88dc41e0 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -56,51 +56,47 @@ int main(int argc, char** argv) QGuiApplication::setHighDpiScaleFactorRoundingPolicy(Qt::HighDpiScaleFactorRoundingPolicy::PassThrough); #endif - Application app(argc, argv); - Application::setApplicationName("KeePassXC"); - Application::setApplicationVersion(KEEPASSXC_VERSION); - app.setProperty("KPXC_QUALIFIED_APPNAME", "org.keepassxc.KeePassXC"); - app.applyTheme(); -#if QT_VERSION >= QT_VERSION_CHECK(5, 7, 0) - QGuiApplication::setDesktopFileName(app.property("KPXC_QUALIFIED_APPNAME").toString() + QStringLiteral(".desktop")); -#endif - - // don't set organizationName as that changes the return value of - // QStandardPaths::writableLocation(QDesktopServices::DataLocation) - Bootstrap::bootstrapApplication(); - QCommandLineParser parser; parser.setApplicationDescription(QObject::tr("KeePassXC - cross-platform password manager")); parser.addPositionalArgument( - "filename", QObject::tr("filenames of the password databases to open (*.kdbx)"), "[filename(s)]"); + "filename(s)", QObject::tr("filenames of the password databases to open (*.kdbx)"), "[filename(s)]"); QCommandLineOption configOption("config", QObject::tr("path to a custom config file"), "config"); + QCommandLineOption localConfigOption( + "localconfig", QObject::tr("path to a custom local config file"), "localconfig"); QCommandLineOption keyfileOption("keyfile", QObject::tr("key file of the database"), "keyfile"); QCommandLineOption pwstdinOption("pw-stdin", QObject::tr("read password of the database from stdin")); - // This is needed under Windows where clients send --parent-window parameter with Native Messaging connect method - QCommandLineOption parentWindowOption(QStringList() << "pw" - << "parent-window", - QObject::tr("Parent window handle"), - "handle"); QCommandLineOption helpOption = parser.addHelpOption(); QCommandLineOption versionOption = parser.addVersionOption(); QCommandLineOption debugInfoOption(QStringList() << "debug-info", QObject::tr("Displays debugging information.")); parser.addOption(configOption); + parser.addOption(localConfigOption); parser.addOption(keyfileOption); parser.addOption(pwstdinOption); - parser.addOption(parentWindowOption); parser.addOption(debugInfoOption); + Application app(argc, argv); + // don't set organizationName as that changes the return value of + // QStandardPaths::writableLocation(QDesktopServices::DataLocation) + Application::setApplicationName("KeePassXC"); + Application::setApplicationVersion(KEEPASSXC_VERSION); + app.setProperty("KPXC_QUALIFIED_APPNAME", "org.keepassxc.KeePassXC"); + parser.process(app); - // Don't try and do anything with the application if we're only showing the help / version + // Exit early if we're only showing the help / version if (parser.isSet(versionOption) || parser.isSet(helpOption)) { return EXIT_SUCCESS; } - const QStringList fileNames = parser.positionalArguments(); + // Process config file options early + if (parser.isSet(configOption) || parser.isSet(localConfigOption)) { + Config::createConfigFromFile(parser.value(configOption), parser.value(localConfigOption)); + } + // Process single instance and early exit if already running + const QStringList fileNames = parser.positionalArguments(); if (app.isAlreadyRunning()) { if (!fileNames.isEmpty()) { app.sendFileNamesToRunningInstance(fileNames); @@ -109,7 +105,14 @@ int main(int argc, char** argv) return EXIT_SUCCESS; } - QApplication::setQuitOnLastWindowClosed(false); + // Apply the configured theme before creating any GUI elements + app.applyTheme(); + +#if QT_VERSION >= QT_VERSION_CHECK(5, 7, 0) + QGuiApplication::setDesktopFileName(app.property("KPXC_QUALIFIED_APPNAME").toString() + QStringLiteral(".desktop")); +#endif + + Bootstrap::bootstrapApplication(); if (!Crypto::init()) { QString error = QObject::tr("Fatal error while testing the cryptographic functions."); @@ -128,10 +131,6 @@ int main(int argc, char** argv) return EXIT_SUCCESS; } - if (parser.isSet(configOption)) { - Config::createConfigFromFile(parser.value(configOption)); - } - MainWindow mainWindow; QObject::connect(&app, SIGNAL(anotherInstanceStarted()), &mainWindow, SLOT(bringToFront())); QObject::connect(&app, SIGNAL(applicationActivated()), &mainWindow, SLOT(bringToFront())); From 829697d53e8d0f3f345a13a97cd5f26f0aaa349c Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sun, 20 Sep 2020 11:28:03 -0400 Subject: [PATCH 14/15] Enforce fixed password font in preview widget * Fix #5432 --- src/gui/EntryPreviewWidget.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/EntryPreviewWidget.cpp b/src/gui/EntryPreviewWidget.cpp index b873800a8..1dc05c3b7 100644 --- a/src/gui/EntryPreviewWidget.cpp +++ b/src/gui/EntryPreviewWidget.cpp @@ -50,7 +50,6 @@ EntryPreviewWidget::EntryPreviewWidget(QWidget* parent) // Entry m_ui->entryTotpButton->setIcon(resources()->icon("chronometer")); m_ui->entryCloseButton->setIcon(resources()->icon("dialog-close")); - m_ui->entryPasswordLabel->setFont(Font::fixedFont()); m_ui->togglePasswordButton->setIcon(resources()->onOffIcon("password-show")); m_ui->toggleEntryNotesButton->setIcon(resources()->onOffIcon("password-show")); m_ui->toggleGroupNotesButton->setIcon(resources()->onOffIcon("password-show")); @@ -194,6 +193,7 @@ void EntryPreviewWidget::setPasswordVisible(bool state) if (state) { m_ui->entryPasswordLabel->setText(password); m_ui->entryPasswordLabel->setCursorPosition(0); + m_ui->entryPasswordLabel->setFont(Font::fixedFont()); } else if (password.isEmpty() && !config()->get(Config::Security_PasswordEmptyPlaceholder).toBool()) { m_ui->entryPasswordLabel->setText(""); } else { From 9fd9d65995e0c1a7ed96fbd025f42b696c40a3d9 Mon Sep 17 00:00:00 2001 From: Bernhard Berg <34011017+Colfenor@users.noreply.github.com> Date: Sat, 26 Sep 2020 15:56:45 +0200 Subject: [PATCH 15/15] reset Qshared ptr (#5470) CLI: Fix heapUseAfterFree in db-create command --- src/cli/keepassxc-cli.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cli/keepassxc-cli.cpp b/src/cli/keepassxc-cli.cpp index 5d326f46e..2f8294446 100644 --- a/src/cli/keepassxc-cli.cpp +++ b/src/cli/keepassxc-cli.cpp @@ -247,6 +247,10 @@ int main(int argc, char** argv) arguments.removeFirst(); int exitCode = command->execute(arguments); + if (command->currentDatabase) { + command->currentDatabase.reset(); + } + #if defined(WITH_ASAN) && defined(WITH_LSAN) // do leak check here to prevent massive tail of end-of-process leak errors from third-party libraries __lsan_do_leak_check();