diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp index 392d28c43..2ec8d7fae 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,20 +601,18 @@ 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)) { + && handleURL(entry->attributes()->value(key), url, submitUrl)) { entries.append(entry); continue; } } } - if (!handleURL(entry->url(), domain, url)) { + if (!handleURL(entry->url(), url, submitUrl)) { continue; } @@ -625,7 +623,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) { @@ -662,7 +660,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)); @@ -1000,7 +998,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; @@ -1017,31 +1015,35 @@ 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) { + if (browserSettings()->matchUrlScheme() && !entryQUrl.scheme().isEmpty() && 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/tests/TestBrowser.cpp b/tests/TestBrowser.cpp index bb3318d07..039d0b15e 100644 --- a/tests/TestBrowser.cpp +++ b/tests/TestBrowser.cpp @@ -179,29 +179,23 @@ 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 +207,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 +220,16 @@ 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 +237,59 @@ 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/{}<>"); - - 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(); - } + 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", + }; + + 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 +297,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 +372,20 @@ 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 = @@ -458,3 +457,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; };