From f73855a7f2168957a1a917ea4154445d8e1aa583 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Sun, 7 Jun 2020 10:22:51 +0300 Subject: [PATCH] Adjust matching with best-matching credentials enabled --- src/browser/BrowserService.cpp | 29 ++++++++--- src/browser/BrowserService.h | 10 ++-- tests/TestBrowser.cpp | 94 +++++++++++++++++++++++++++++----- tests/TestBrowser.h | 1 + 4 files changed, 113 insertions(+), 21 deletions(-) diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp index 1f54e33ca..e0b8dacc2 100644 --- a/src/browser/BrowserService.cpp +++ b/src/browser/BrowserService.cpp @@ -413,7 +413,7 @@ QJsonArray BrowserService::findMatchingEntries(const QString& dbid, } // Sort results - pwEntries = sortEntries(pwEntries, host, submitUrl); + pwEntries = sortEntries(pwEntries, host, submitUrl, url); // Fill the list QJsonArray result; @@ -698,7 +698,10 @@ void BrowserService::convertAttributesToCustomData(QSharedPointer db) } } -QList BrowserService::sortEntries(QList& pwEntries, const QString& host, const QString& entryUrl) +QList BrowserService::sortEntries(QList& pwEntries, + const QString& host, + const QString& entryUrl, + const QString& fullUrl) { QUrl url(entryUrl); if (url.scheme().isEmpty()) { @@ -712,7 +715,7 @@ QList BrowserService::sortEntries(QList& pwEntries, const QStrin // Build map of prioritized entries QMultiMap priorities; for (auto* entry : pwEntries) { - priorities.insert(sortPriority(entry, host, submitUrl, baseSubmitUrl), entry); + priorities.insert(sortPriority(entry, host, submitUrl, baseSubmitUrl, fullUrl), entry); } QList results; @@ -895,7 +898,8 @@ Group* BrowserService::getDefaultEntryGroup(const QSharedPointer& sele int BrowserService::sortPriority(const Entry* entry, const QString& host, const QString& submitUrl, - const QString& baseSubmitUrl) const + const QString& baseSubmitUrl, + const QString& fullUrl) const { QUrl url(entry->url()); if (url.scheme().isEmpty()) { @@ -914,9 +918,12 @@ int BrowserService::sortPriority(const Entry* entry, if (!url.host().contains(".") && url.host() != "localhost") { return 0; } - if (submitUrl == entryURL) { + if (fullUrl == entryURL) { return 100; } + if (submitUrl == entryURL) { + return 95; + } if (submitUrl.startsWith(entryURL) && entryURL != host && baseSubmitUrl != entryURL) { return 90; } @@ -1025,7 +1032,17 @@ bool BrowserService::handleURL(const QString& entryUrl, const QString& url, cons // Match the subdomains with the limited wildcard if (siteQUrl.host().endsWith(entryQUrl.host())) { - return true; + 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 false; diff --git a/src/browser/BrowserService.h b/src/browser/BrowserService.h index 6567a44d0..77635cfe1 100644 --- a/src/browser/BrowserService.h +++ b/src/browser/BrowserService.h @@ -119,7 +119,8 @@ private: 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); + QList + sortEntries(QList& pwEntries, const QString& host, const QString& submitUrl, const QString& fullUrl); QList confirmEntries(QList& pwEntriesToConfirm, const QString& url, const QString& host, @@ -130,8 +131,11 @@ private: QJsonArray getChildrenFromGroup(Group* group); Access checkAccess(const Entry* entry, const QString& host, const QString& submitHost, const QString& realm); Group* getDefaultEntryGroup(const QSharedPointer& selectedDb = {}); - int - sortPriority(const Entry* entry, const QString& host, const QString& submitUrl, const QString& baseSubmitUrl) const; + int sortPriority(const Entry* entry, + const QString& host, + const QString& submitUrl, + const QString& baseSubmitUrl, + const QString& fullUrl) const; bool schemeFound(const QString& url); bool removeFirstDomain(QString& hostname); bool handleURL(const QString& entryUrl, const QString& url, const QString& submitUrl); diff --git a/tests/TestBrowser.cpp b/tests/TestBrowser.cpp index 5b2f61178..3e518c1e2 100644 --- a/tests/TestBrowser.cpp +++ b/tests/TestBrowser.cpp @@ -38,6 +38,7 @@ void TestBrowser::initTestCase() { QVERIFY(Crypto::init()); m_browserService = browserService(); + browserSettings()->setBestMatchOnly(false); } void TestBrowser::init() @@ -130,6 +131,7 @@ void TestBrowser::testSortPriority() QString host = "github.com"; QString submitUrl = "https://github.com/session"; QString baseSubmitUrl = "https://github.com"; + QString fullUrl = "https://github.com/login"; QScopedPointer entry1(new Entry()); QScopedPointer entry2(new Entry()); @@ -141,6 +143,7 @@ void TestBrowser::testSortPriority() QScopedPointer entry8(new Entry()); QScopedPointer entry9(new Entry()); QScopedPointer entry10(new Entry()); + QScopedPointer entry11(new Entry()); entry1->setUrl("https://github.com/login"); entry2->setUrl("https://github.com/login"); @@ -152,18 +155,20 @@ void TestBrowser::testSortPriority() entry8->setUrl("github.com/login"); entry9->setUrl("https://github"); // Invalid URL entry10->setUrl("github.com"); + entry11->setUrl("https://github.com/login"); // Exact match // The extension uses the submitUrl as default for comparison - auto res1 = m_browserService->sortPriority(entry1.data(), host, "https://github.com/login", baseSubmitUrl); - auto res2 = m_browserService->sortPriority(entry2.data(), host, submitUrl, baseSubmitUrl); - auto res3 = m_browserService->sortPriority(entry3.data(), host, submitUrl, baseSubmitUrl); - auto res4 = m_browserService->sortPriority(entry4.data(), host, submitUrl, baseSubmitUrl); - auto res5 = m_browserService->sortPriority(entry5.data(), host, submitUrl, baseSubmitUrl); - auto res6 = m_browserService->sortPriority(entry6.data(), host, submitUrl, baseSubmitUrl); - auto res7 = m_browserService->sortPriority(entry7.data(), host, submitUrl, baseSubmitUrl); - auto res8 = m_browserService->sortPriority(entry8.data(), host, submitUrl, baseSubmitUrl); - auto res9 = m_browserService->sortPriority(entry9.data(), host, submitUrl, baseSubmitUrl); - auto res10 = m_browserService->sortPriority(entry10.data(), host, submitUrl, baseSubmitUrl); + 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); QCOMPARE(res1, 100); QCOMPARE(res2, 40); @@ -175,6 +180,7 @@ void TestBrowser::testSortPriority() QCOMPARE(res8, 0); QCOMPARE(res9, 0); QCOMPARE(res10, 0); + QCOMPARE(res11, 100); } void TestBrowser::testSearchEntries() @@ -382,8 +388,8 @@ void TestBrowser::testSortEntries() auto entries = createEntries(urls, root); browserSettings()->setBestMatchOnly(false); - auto result = - m_browserService->sortEntries(entries, "github.com", "https://github.com/session"); // entries, host, submitUrl + auto result = m_browserService->sortEntries( + entries, "github.com", "https://github.com/session", "https://github.com"); // entries, host, submitUrl QCOMPARE(result.size(), 10); QCOMPARE(result[0]->username(), QString("User 2")); QCOMPARE(result[0]->url(), QString("https://github.com/")); @@ -393,6 +399,15 @@ void TestBrowser::testSortEntries() QCOMPARE(result[2]->url(), QString("https://github.com/login")); QCOMPARE(result[3]->username(), QString("User 3")); QCOMPARE(result[3]->url(), QString("github.com/login")); + + // 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"); + 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/")); } QList TestBrowser::createEntries(QStringList& urls, Group* root) const @@ -429,3 +444,58 @@ void TestBrowser::testValidURLs() QCOMPARE(Tools::checkUrlValid(i.key()), i.value()); } } + +void TestBrowser::testBestMatchingCredentials() +{ + auto db = QSharedPointer::create(); + auto* root = db->rootGroup(); + + // Test with simple URL entries + QStringList urls = {"https://github.com/loginpage", "https://github.com/justsomepage", "https://github.com/"}; + + auto entries = createEntries(urls, root); + + 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")); + + 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")); + + 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/")); + + 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")); + + // Test with subdomains + QStringList subdomainsUrls = {"https://sub.github.com/loginpage", + "https://sub.github.com/justsomepage", + "https://bus.github.com/justsomepage"}; + + entries = createEntries(subdomainsUrls, root); + + browserSettings()->setBestMatchOnly(true); + + 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")); + + 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")); + + 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")); +} diff --git a/tests/TestBrowser.h b/tests/TestBrowser.h index 00f9d7528..c8be3d6ca 100644 --- a/tests/TestBrowser.h +++ b/tests/TestBrowser.h @@ -47,6 +47,7 @@ private slots: void testSubdomainsAndPaths(); void testSortEntries(); void testValidURLs(); + void testBestMatchingCredentials(); private: QList createEntries(QStringList& urls, Group* root) const;