Skip to content

Commit

Permalink
Move wildcard support to Additional URLs only
Browse files Browse the repository at this point in the history
  • Loading branch information
varjolintu committed Jan 21, 2025
1 parent 9114eb2 commit a20fa8e
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 87 deletions.
42 changes: 27 additions & 15 deletions src/browser/BrowserService.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2024 KeePassXC Team <[email protected]>
* Copyright (C) 2025 KeePassXC Team <[email protected]>
* Copyright (C) 2017 Sami Vänttinen <[email protected]>
* Copyright (C) 2013 Francois Ferrand
*
Expand Down Expand Up @@ -51,6 +51,7 @@
#include <QLocalSocket>
#include <QLocale>
#include <QProgressDialog>
#include <QStringView>
#include <QUrl>

const QString BrowserService::KEEPASSXCBROWSER_NAME = QStringLiteral("KeePassXC-Browser Settings");
Expand Down Expand Up @@ -1375,9 +1376,15 @@ bool BrowserService::shouldIncludeEntry(Entry* entry,
return url.endsWith("by-path/" + entry->path());
}

const auto allEntryUrls = entry->getAllUrls();
for (const auto& entryUrl : allEntryUrls) {
if (handleURL(entryUrl, url, submitUrl, omitWwwSubdomain)) {
// Handle the entry URL
if (handleURL(entry->resolveUrl(), url, submitUrl, omitWwwSubdomain)) {
return true;
}

// Handle additional URLs
const auto additionalUrls = entry->getAdditionalUrls();
for (const auto& additionalUrl : additionalUrls) {
if (handleURL(additionalUrl, url, submitUrl, omitWwwSubdomain, true)) {
return true;
}
}
Expand Down Expand Up @@ -1465,23 +1472,28 @@ QJsonObject BrowserService::getPasskeyError(int errorCode) const
bool BrowserService::handleURL(const QString& entryUrl,
const QString& siteUrl,
const QString& formUrl,
const bool omitWwwSubdomain)
const bool omitWwwSubdomain,
const bool allowWildcards)
{
if (entryUrl.isEmpty()) {
return false;
}

// Exact match where URL is wrapped inside " characters
if (entryUrl.startsWith("\"") && entryUrl.endsWith("\"") && entryUrl.midRef(1, entryUrl.length() - 2) == siteUrl) {
return true;
}
bool isWildcardUrl = false;
auto tempUrl = entryUrl;

const auto isWildcardUrl = entryUrl.contains("*");
// Allows matching with exact URL and wildcards
if (allowWildcards) {
// Exact match where URL is wrapped inside " characters
if (entryUrl.startsWith("\"") && entryUrl.endsWith("\"")) {
return QStringView{entryUrl}.mid(1, entryUrl.length() - 2) == siteUrl;
}

// Replace wildcards
auto tempUrl = entryUrl;
if (isWildcardUrl) {
tempUrl = tempUrl.replace("*", UrlTools::URL_WILDCARD);
// Replace wildcards
isWildcardUrl = entryUrl.contains("*");
if (isWildcardUrl) {
tempUrl = tempUrl.replace("*", UrlTools::URL_WILDCARD);
}
}

QUrl entryQUrl;
Expand Down Expand Up @@ -1512,7 +1524,7 @@ bool BrowserService::handleURL(const QString& entryUrl,

// Match port, if used
QUrl siteQUrl(siteUrl);
if ((entryQUrl.port() > 0) && entryQUrl.port() != siteQUrl.port()) {
if (entryQUrl.port() > 0 && entryQUrl.port() != siteQUrl.port()) {
return false;
}

Expand Down
5 changes: 3 additions & 2 deletions src/browser/BrowserService.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2024 KeePassXC Team <[email protected]>
* Copyright (C) 2025 KeePassXC Team <[email protected]>
* Copyright (C) 2017 Sami Vänttinen <[email protected]>
* Copyright (C) 2013 Francois Ferrand
*
Expand Down Expand Up @@ -200,7 +200,8 @@ private slots:
bool handleURL(const QString& entryUrl,
const QString& siteUrl,
const QString& formUrl,
const bool omitWwwSubdomain = false);
const bool omitWwwSubdomain = false,
const bool allowWildcards = false);
bool handleURLWithWildcards(const QUrl& entryQUrl, const QString& siteUrl);
QString getDatabaseRootUuid();
QString getDatabaseRecycleBinUuid();
Expand Down
22 changes: 19 additions & 3 deletions src/core/Entry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -381,16 +381,32 @@ QString Entry::url() const
return m_attributes->value(EntryAttributes::URLKey);
}

QString Entry::resolveUrl() const
{
const auto entryUrl = url();
if (entryUrl.isEmpty()) {
return {};
}

return EntryAttributes::matchReference(entryUrl).hasMatch() ? resolveMultiplePlaceholders(entryUrl) : entryUrl;
}

QStringList Entry::getAllUrls() const
{
QStringList urlList;
auto entryUrl = url();

const auto entryUrl = resolveUrl();
if (!entryUrl.isEmpty()) {
urlList << (EntryAttributes::matchReference(entryUrl).hasMatch() ? resolveMultiplePlaceholders(entryUrl)
: entryUrl);
urlList << entryUrl;
}

return urlList << getAdditionalUrls();
}

QStringList Entry::getAdditionalUrls() const
{
QStringList urlList;

for (const auto& key : m_attributes->keys()) {
if (key.startsWith(EntryAttributes::AdditionalUrlAttribute)
|| key == QString("%1_RELYING_PARTY").arg(EntryAttributes::PasskeyAttribute)) {
Expand Down
4 changes: 3 additions & 1 deletion src/core/Entry.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2024 KeePassXC Team <[email protected]>
* Copyright (C) 2025 KeePassXC Team <[email protected]>
* Copyright (C) 2010 Felix Geyer <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
Expand Down Expand Up @@ -100,7 +100,9 @@ class Entry : public ModifiableObject
const AutoTypeAssociations* autoTypeAssociations() const;
QString title() const;
QString url() const;
QString resolveUrl() const;
QStringList getAllUrls() const;
QStringList getAdditionalUrls() const;
QString webUrl() const;
QString displayUrl() const;
QString username() const;
Expand Down
141 changes: 77 additions & 64 deletions tests/TestBrowser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,27 +197,24 @@ void TestBrowser::testSearchEntries()
auto db = QSharedPointer<Database>::create();
auto* root = db->rootGroup();

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",
"\"https://github.com\"" // Exact URL
};
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"};

createEntries(urls, root);

browserSettings()->setMatchUrlScheme(false);
auto result =
m_browserService->searchEntries(db, "https://github.com", "https://github.com/session"); // db, url, submitUrl

QCOMPARE(result.length(), 10);
QCOMPARE(result.length(), 9);
QCOMPARE(result[0]->url(), QString("https://github.com/login_page"));
QCOMPARE(result[1]->url(), QString("https://github.com/login"));
QCOMPARE(result[2]->url(), QString("https://github.com/"));
Expand All @@ -228,7 +225,7 @@ void TestBrowser::testSearchEntries()
// With matching there should be only 4 results + 4 without a scheme
browserSettings()->setMatchUrlScheme(true);
result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session");
QCOMPARE(result.length(), 8);
QCOMPARE(result.length(), 7);
QCOMPARE(result[0]->url(), QString("https://github.com/login_page"));
QCOMPARE(result[1]->url(), QString("https://github.com/login"));
QCOMPARE(result[2]->url(), QString("https://github.com/"));
Expand Down Expand Up @@ -404,102 +401,114 @@ void TestBrowser::testSearchEntriesWithWildcardURLs()
auto db = QSharedPointer<Database>::create();
auto* root = db->rootGroup();

QStringList urls = {"https://github.com/login_page/*",
"https://github.com/*/second",
"https://github.com/*",
"http://github.com/*",
"github.com/*", // Defaults to https
"https://*.github.com/*",
"https://subdomain.*.github.com/*/second",
"https://*.sub.github.com/*",
"https://********", // Invalid wildcard URL
"https://subdomain.yes.github.com/*",
"https://example.com:8448/*",
"https://example.com/*/*",
"https://example.com/$/*",
"https://127.128.129.*:8448/",
"https://127.128.*/",
"https://127.160.*.2/login",
"http://[2001:db8:85a3:8d3:1319:8a2e:370:*]/",
"https://[2001:db8:85a3:8d3:*]:443/",
"fe80::1ff:fe23:4567:890a",
"2001-db8-85a3-8d3-1319-8a2e-370-7348.ipv6-literal.net"};
QStringList urls = {
"https://github.com/login_page/*",
"https://github.com/*/second",
"https://github.com/*",
"http://github.com/*",
"github.com/*", // Defaults to https
"https://*.github.com/*",
"https://subdomain.*.github.com/*/second",
"https://*.sub.github.com/*",
"https://********", // Invalid wildcard URL
"https://subdomain.yes.github.com/*",
"https://example.com:8448/*",
"https://example.com/*/*",
"https://example.com/$/*",
"https://127.128.129.*:8448/",
"https://127.128.*/",
"https://127.160.*.2/login",
"http://[2001:db8:85a3:8d3:1319:8a2e:370:*]/",
"https://[2001:db8:85a3:8d3:*]:443/",
"fe80::1ff:fe23:4567:890a",
"2001-db8-85a3-8d3-1319-8a2e-370-7348.ipv6-literal.net",
"\"https://thisisatest.com/login.php\"" // Exact URL
};

createEntries(urls, root);
createEntries(urls, root, true);
browserSettings()->setMatchUrlScheme(false);

// Return first Additional URL
auto firstUrl = [&](Entry* entry) { return entry->attributes()->value(EntryAttributes::AdditionalUrlAttribute); };

auto result = m_browserService->searchEntries(
db, "https://github.com/login_page/second", "https://github.com/login_page/second");
QCOMPARE(result.length(), 6);
QCOMPARE(result[0]->url(), QString("https://github.com/login_page/*"));
QCOMPARE(result[1]->url(), QString("https://github.com/*/second"));
QCOMPARE(result[2]->url(), QString("https://github.com/*"));
QCOMPARE(result[3]->url(), QString("http://github.com/*"));
QCOMPARE(result[4]->url(), QString("github.com/*"));
QCOMPARE(result[5]->url(), QString("https://*.github.com/*"));
QCOMPARE(firstUrl(result[0]), QString("https://github.com/login_page/*"));
QCOMPARE(firstUrl(result[1]), QString("https://github.com/*/second"));
QCOMPARE(firstUrl(result[2]), QString("https://github.com/*"));
QCOMPARE(firstUrl(result[3]), QString("http://github.com/*"));
QCOMPARE(firstUrl(result[4]), QString("github.com/*"));
QCOMPARE(firstUrl(result[5]), QString("https://*.github.com/*"));

result = m_browserService->searchEntries(
db, "https://subdomain.sub.github.com/login_page/second", "https://subdomain.sub.github.com/login_page/second");
QCOMPARE(result.length(), 3);
QCOMPARE(result[0]->url(), QString("https://*.github.com/*"));
QCOMPARE(result[1]->url(), QString("https://subdomain.*.github.com/*/second"));
QCOMPARE(result[2]->url(), QString("https://*.sub.github.com/*"));
QCOMPARE(firstUrl(result[0]), QString("https://*.github.com/*"));
QCOMPARE(firstUrl(result[1]), QString("https://subdomain.*.github.com/*/second"));
QCOMPARE(firstUrl(result[2]), QString("https://*.sub.github.com/*"));

result = m_browserService->searchEntries(
db, "https://subdomain.sub.github.com/other_page", "https://subdomain.sub.github.com/other_page");
QCOMPARE(result.length(), 2);
QCOMPARE(result[0]->url(), QString("https://*.github.com/*"));
QCOMPARE(result[1]->url(), QString("https://*.sub.github.com/*"));
QCOMPARE(firstUrl(result[0]), QString("https://*.github.com/*"));
QCOMPARE(firstUrl(result[1]), QString("https://*.sub.github.com/*"));

result = m_browserService->searchEntries(
db, "https://subdomain.yes.github.com/other_page/second", "https://subdomain.yes.github.com/other_page/second");
QCOMPARE(result.length(), 3);
QCOMPARE(result[0]->url(), QString("https://*.github.com/*"));
QCOMPARE(result[1]->url(), QString("https://subdomain.*.github.com/*/second"));
QCOMPARE(result[2]->url(), QString("https://subdomain.yes.github.com/*"));
QCOMPARE(firstUrl(result[0]), QString("https://*.github.com/*"));
QCOMPARE(firstUrl(result[1]), QString("https://subdomain.*.github.com/*/second"));
QCOMPARE(firstUrl(result[2]), QString("https://subdomain.yes.github.com/*"));

result = m_browserService->searchEntries(
db, "https://example.com:8448/login/page", "https://example.com:8448/login/page");
QCOMPARE(result.length(), 2);
QCOMPARE(result[0]->url(), QString("https://example.com:8448/*"));
QCOMPARE(result[1]->url(), QString("https://example.com/*/*"));
QCOMPARE(firstUrl(result[0]), QString("https://example.com:8448/*"));
QCOMPARE(firstUrl(result[1]), QString("https://example.com/*/*"));

result = m_browserService->searchEntries(
db, "https://example.com:8449/login/page", "https://example.com:8449/login/page");
QCOMPARE(result.length(), 1);
QCOMPARE(result[0]->url(), QString("https://example.com/*/*"));
QCOMPARE(firstUrl(result[0]), QString("https://example.com/*/*"));

result =
m_browserService->searchEntries(db, "https://example.com/$/login_page", "https://example.com/$/login_page");
QCOMPARE(result.length(), 2);
QCOMPARE(result[0]->url(), QString("https://example.com/*/*"));
QCOMPARE(result[1]->url(), QString("https://example.com/$/*"));
QCOMPARE(firstUrl(result[0]), QString("https://example.com/*/*"));
QCOMPARE(firstUrl(result[1]), QString("https://example.com/$/*"));

result = m_browserService->searchEntries(db, "https://127.128.129.130:8448/", "https://127.128.129.130:8448/");
QCOMPARE(result.length(), 2);

result = m_browserService->searchEntries(db, "https://127.128.129.130/", "https://127.128.129.130/");
QCOMPARE(result.length(), 1);
QCOMPARE(result[0]->url(), QString("https://127.128.*/"));
QCOMPARE(firstUrl(result[0]), QString("https://127.128.*/"));

result = m_browserService->searchEntries(db, "https://127.1.129.130/", "https://127.1.129.130/");
QCOMPARE(result.length(), 0);

result = m_browserService->searchEntries(db, "https://127.160.8.2/login", "https://127.160.8.2/login");
QCOMPARE(result.length(), 1);
QCOMPARE(result[0]->url(), QString("https://127.160.*.2/login"));
QCOMPARE(firstUrl(result[0]), QString("https://127.160.*.2/login"));

// Exact URL
result = m_browserService->searchEntries(
db, "https://thisisatest.com/login.php", "https://thisisatest.com/login.php");
QCOMPARE(result.length(), 1);
QCOMPARE(firstUrl(result[0]), QString("\"https://thisisatest.com/login.php\""));

// With scheme matching enabled
browserSettings()->setMatchUrlScheme(true);
result = m_browserService->searchEntries(
db, "https://github.com/login_page/second", "https://github.com/login_page/second");

QCOMPARE(result.length(), 5);
QCOMPARE(result[0]->url(), QString("https://github.com/login_page/*"));
QCOMPARE(result[1]->url(), QString("https://github.com/*/second"));
QCOMPARE(result[2]->url(), QString("https://github.com/*"));
QCOMPARE(result[3]->url(), QString("github.com/*")); // Defaults to https
QCOMPARE(result[4]->url(), QString("https://*.github.com/*"));
QCOMPARE(firstUrl(result[0]), QString("https://github.com/login_page/*"));
QCOMPARE(firstUrl(result[1]), QString("https://github.com/*/second"));
QCOMPARE(firstUrl(result[2]), QString("https://github.com/*"));
QCOMPARE(firstUrl(result[3]), QString("github.com/*")); // Defaults to https
QCOMPARE(firstUrl(result[4]), QString("https://*.github.com/*"));
}

void TestBrowser::testInvalidEntries()
Expand Down Expand Up @@ -622,14 +631,18 @@ void TestBrowser::testSubdomainsAndPaths()
QCOMPARE(result.length(), 1);
}

QList<Entry*> TestBrowser::createEntries(QStringList& urls, Group* root) const
QList<Entry*> TestBrowser::createEntries(QStringList& urls, Group* root, bool additionalUrl) const
{
QList<Entry*> entries;
for (int i = 0; i < urls.length(); ++i) {
auto entry = new Entry();
entry->setGroup(root);
entry->beginUpdate();
entry->setUrl(urls[i]);
if (additionalUrl) {
entry->attributes()->set(EntryAttributes::AdditionalUrlAttribute, urls[i]);
} else {
entry->setUrl(urls[i]);
}
entry->setUsername(QString("User %1").arg(i));
entry->setUuid(QUuid::createUuid());
entry->setTitle(QString("Name_%1").arg(entry->uuidToHex()));
Expand Down
4 changes: 2 additions & 2 deletions tests/TestBrowser.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2023 KeePassXC Team <[email protected]>
* Copyright (C) 2025 KeePassXC Team <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -53,7 +53,7 @@ private slots:
void testRestrictBrowserKey();

private:
QList<Entry*> createEntries(QStringList& urls, Group* root) const;
QList<Entry*> createEntries(QStringList& urls, Group* root, bool additionalUrl = false) const;
void compareEntriesByPath(QSharedPointer<Database> db, QList<Entry*> entries, QString path);

QScopedPointer<BrowserAction> m_browserAction;
Expand Down

0 comments on commit a20fa8e

Please sign in to comment.