Skip to content

Commit

Permalink
Allow groups to restrict by browser integration key (#6437)
Browse files Browse the repository at this point in the history
  • Loading branch information
ekimd committed Sep 18, 2023
1 parent 27c5c5d commit 42fbf09
Show file tree
Hide file tree
Showing 10 changed files with 207 additions and 9 deletions.
8 changes: 8 additions & 0 deletions share/translations/keepassxc_en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3133,6 +3133,14 @@ Would you like to correct it?</source>
<source>Omit WWW subdomain from matching toggle for this and sub groups</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Restrict matching to given browser key:</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Restrict matching to given browser key toggle for this and sub groups</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>EditGroupWidgetKeeShare</name>
Expand Down
38 changes: 30 additions & 8 deletions src/browser/BrowserService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const QString BrowserService::OPTION_HIDE_ENTRY = QStringLiteral("BrowserHideEnt
const QString BrowserService::OPTION_ONLY_HTTP_AUTH = QStringLiteral("BrowserOnlyHttpAuth");
const QString BrowserService::OPTION_NOT_HTTP_AUTH = QStringLiteral("BrowserNotHttpAuth");
const QString BrowserService::OPTION_OMIT_WWW = QStringLiteral("BrowserOmitWww");
const QString BrowserService::OPTION_RESTRICT_KEY = QStringLiteral("BrowserRestrictKey");
// Multiple URL's
const QString BrowserService::ADDITIONAL_URL = QStringLiteral("KP2A_URL");

Expand Down Expand Up @@ -773,8 +774,10 @@ bool BrowserService::deleteEntry(const QString& uuid)
return true;
}

QList<Entry*>
BrowserService::searchEntries(const QSharedPointer<Database>& db, const QString& siteUrl, const QString& formUrl)
QList<Entry*> BrowserService::searchEntries(const QSharedPointer<Database>& db,
const QString& siteUrl,
const QString& formUrl,
const QList<QString>& keys)
{
QList<Entry*> entries;
auto* rootGroup = db->rootGroup();
Expand All @@ -788,6 +791,12 @@ BrowserService::searchEntries(const QSharedPointer<Database>& db, const QString&
continue;
}

// If a key restriction is specified and not contained in the keys list then skip this group.
QString restrictKey = group->resolveCustomDataString(BrowserService::OPTION_RESTRICT_KEY);
if (!restrictKey.isEmpty() && !keys.contains(restrictKey)) {
continue;
}

const auto omitWwwSubdomain =
group->resolveCustomDataTriState(BrowserService::OPTION_OMIT_WWW) == Group::Enable;

Expand Down Expand Up @@ -815,30 +824,35 @@ BrowserService::searchEntries(const QSharedPointer<Database>& db, const QString&
QList<Entry*>
BrowserService::searchEntries(const QString& siteUrl, const QString& formUrl, const StringPairList& keyList)
{
// Check if database is connected with KeePassXC-Browser
// Check if database is connected with KeePassXC-Browser. If so, return browser key (otherwise empty)
auto databaseConnected = [&](const QSharedPointer<Database>& db) {
for (const StringPair& keyPair : keyList) {
QString key = db->metadata()->customData()->value(CustomData::BrowserKeyPrefix + keyPair.first);
if (!key.isEmpty() && keyPair.second == key) {
return true;
return keyPair.first;
}
}
return false;
return QString();
};

// Get the list of databases to search
QList<QSharedPointer<Database>> databases;
QList<QString> keys;
if (browserSettings()->searchInAllDatabases()) {
for (auto dbWidget : getMainWindow()->getOpenDatabases()) {
auto db = dbWidget->database();
if (db && databaseConnected(dbWidget->database())) {
auto key = databaseConnected(dbWidget->database());
if (db && !key.isEmpty()) {
databases << db;
keys << key;
}
}
} else {
const auto& db = getDatabase();
if (databaseConnected(db)) {
auto key = databaseConnected(db);
if (!key.isEmpty()) {
databases << db;
keys << key;
}
}

Expand All @@ -847,13 +861,21 @@ BrowserService::searchEntries(const QString& siteUrl, const QString& formUrl, co
QList<Entry*> entries;
do {
for (const auto& db : databases) {
entries << searchEntries(db, siteUrl, formUrl);
entries << searchEntries(db, siteUrl, formUrl, keys);
}
} while (entries.isEmpty() && removeFirstDomain(hostname));

return entries;
}

QString BrowserService::decodeCustomDataRestrictKey(const QString& key)
{
if (key.isEmpty())
return tr("Disable");
else
return key;
}

void BrowserService::requestGlobalAutoType(const QString& search)
{
emit osUtils->globalShortcutTriggered("autotype", search);
Expand Down
8 changes: 7 additions & 1 deletion src/browser/BrowserService.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,16 @@ class BrowserService : public QObject
QJsonArray findEntries(const EntryParameters& entryParameters, const StringPairList& keyList, bool* entriesFound);
void requestGlobalAutoType(const QString& search);

static QString decodeCustomDataRestrictKey(const QString& key);

static const QString KEEPASSXCBROWSER_NAME;
static const QString KEEPASSXCBROWSER_OLD_NAME;
static const QString OPTION_SKIP_AUTO_SUBMIT;
static const QString OPTION_HIDE_ENTRY;
static const QString OPTION_ONLY_HTTP_AUTH;
static const QString OPTION_NOT_HTTP_AUTH;
static const QString OPTION_OMIT_WWW;
static const QString OPTION_RESTRICT_KEY;
static const QString ADDITIONAL_URL;

signals:
Expand Down Expand Up @@ -130,7 +133,10 @@ private slots:
Hidden
};

QList<Entry*> searchEntries(const QSharedPointer<Database>& db, const QString& siteUrl, const QString& formUrl);
QList<Entry*> searchEntries(const QSharedPointer<Database>& db,
const QString& siteUrl,
const QString& formUrl,
const QList<QString>& keys = QList<QString>());
QList<Entry*> searchEntries(const QString& siteUrl, const QString& formUrl, const StringPairList& keyList);
QList<Entry*> sortEntries(QList<Entry*>& entries, const QString& siteUrl, const QString& formUrl);
QList<Entry*> confirmEntries(QList<Entry*>& entriesToConfirm,
Expand Down
15 changes: 15 additions & 0 deletions src/core/Group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,21 @@ void Group::setCustomDataTriState(const QString& key, const Group::TriState& val
}
}

// Note that this returns an empty string both if the key is missing *or* if the key is present but value is empty.
QString Group::resolveCustomDataString(const QString& key, bool checkParent) const
{
// If not defined, check our parent up to the root group
if (!m_customData->contains(key)) {
if (!m_parent || !checkParent) {
return QString();
} else {
return m_parent->resolveCustomDataString(key);
}
}

return m_customData->value(key);
}

bool Group::equals(const Group* other, CompareItemOptions options) const
{
if (!other) {
Expand Down
1 change: 1 addition & 0 deletions src/core/Group.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class Group : public ModifiableObject
const CustomData* customData() const;
Group::TriState resolveCustomDataTriState(const QString& key, bool checkParent = true) const;
void setCustomDataTriState(const QString& key, const Group::TriState& value);
QString resolveCustomDataString(const QString& key, bool checkParent = true) const;
const Group* previousParentGroup() const;
QUuid previousParentGroupUuid() const;

Expand Down
58 changes: 58 additions & 0 deletions src/gui/group/EditGroupWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ void EditGroupWidget::loadGroup(Group* group, bool create, const QSharedPointer<
auto inheritOnlyHttp = false;
auto inheritNoHttp = false;
auto inheritOmitWww = false;
auto inheritRestrictKey = QString();

auto parent = group->parentGroup();
if (parent) {
Expand All @@ -204,6 +205,7 @@ void EditGroupWidget::loadGroup(Group* group, bool create, const QSharedPointer<
inheritOnlyHttp = parent->resolveCustomDataTriState(BrowserService::OPTION_ONLY_HTTP_AUTH);
inheritNoHttp = parent->resolveCustomDataTriState(BrowserService::OPTION_NOT_HTTP_AUTH);
inheritOmitWww = parent->resolveCustomDataTriState(BrowserService::OPTION_OMIT_WWW);
inheritRestrictKey = parent->resolveCustomDataString(BrowserService::OPTION_RESTRICT_KEY);
}

// If the page has not been created at all, some of the elements are null
Expand All @@ -219,6 +221,7 @@ void EditGroupWidget::loadGroup(Group* group, bool create, const QSharedPointer<
addTriStateItems(m_browserUi->browserIntegrationOnlyHttpAuthComboBox, inheritOnlyHttp);
addTriStateItems(m_browserUi->browserIntegrationNotHttpAuthComboBox, inheritNoHttp);
addTriStateItems(m_browserUi->browserIntegrationOmitWwwCombobox, inheritOmitWww);
addRestrictKeyComboBoxItems(m_db->metadata()->customData()->keys(), inheritRestrictKey);

m_browserUi->browserIntegrationHideEntriesComboBox->setCurrentIndex(
indexFromTriState(group->resolveCustomDataTriState(BrowserService::OPTION_HIDE_ENTRY, false)));
Expand All @@ -230,6 +233,7 @@ void EditGroupWidget::loadGroup(Group* group, bool create, const QSharedPointer<
indexFromTriState(group->resolveCustomDataTriState(BrowserService::OPTION_NOT_HTTP_AUTH, false)));
m_browserUi->browserIntegrationOmitWwwCombobox->setCurrentIndex(
indexFromTriState(group->resolveCustomDataTriState(BrowserService::OPTION_OMIT_WWW, false)));
setRestrictKeyComboBoxIndex(group);
} else if (hasPage(m_browserWidget)) {
setPageHidden(m_browserWidget, true);
}
Expand Down Expand Up @@ -303,6 +307,7 @@ void EditGroupWidget::apply()
m_temporaryGroup->setCustomDataTriState(
BrowserService::OPTION_OMIT_WWW,
triStateFromIndex(m_browserUi->browserIntegrationOmitWwwCombobox->currentIndex()));
setRestrictKeyCustomData(m_temporaryGroup->customData());
}
#endif

Expand Down Expand Up @@ -444,3 +449,56 @@ Group::TriState EditGroupWidget::triStateFromIndex(int index)
return Group::Inherit;
}
}

void EditGroupWidget::addRestrictKeyComboBoxItems(QList<QString> const& keyList, QString inheritValue)
{
auto comboBox = m_browserUi->browserIntegrationRestrictKeyCombobox;

comboBox->clear();
comboBox->addItem(
tr("Inherit from parent group (%1)").arg(BrowserService::decodeCustomDataRestrictKey(inheritValue)));
comboBox->addItem(tr("Disable"));

comboBox->insertSeparator(2);

// Add all the browser keys to the combobox
for (const QString& key : keyList) {
if (key.startsWith(CustomData::BrowserKeyPrefix)) {
QString strippedKey = key;
strippedKey.remove(CustomData::BrowserKeyPrefix);
comboBox->addItem(strippedKey);
}
}
}

void EditGroupWidget::setRestrictKeyComboBoxIndex(const Group* group)
{
auto comboBox = m_browserUi->browserIntegrationRestrictKeyCombobox;

if (!group || !group->customData()->contains(BrowserService::OPTION_RESTRICT_KEY)) {
comboBox->setCurrentIndex(0);
return;
}

auto key = group->customData()->value(BrowserService::OPTION_RESTRICT_KEY);
if (key.isEmpty()) {
comboBox->setCurrentIndex(1);
} else {
comboBox->setCurrentText(key);
}
}

// Set the customData regarding OPTION_RESTRICT_KEY
void EditGroupWidget::setRestrictKeyCustomData(CustomData* customData)
{
auto comboBox = m_browserUi->browserIntegrationRestrictKeyCombobox;
auto key = BrowserService::OPTION_RESTRICT_KEY;
auto idx = comboBox->currentIndex();
if (idx == 0) {
customData->remove(key);
} else if (idx == 1) {
customData->set(key, QString());
} else {
customData->set(key, comboBox->currentText());
}
}
4 changes: 4 additions & 0 deletions src/gui/group/EditGroupWidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ private slots:
Group::TriState triStateFromIndex(int index);
void setupModifiedTracking();

void addRestrictKeyComboBoxItems(QList<QString> const& keyList, QString inheritValue);
void setRestrictKeyComboBoxIndex(const Group* group);
void setRestrictKeyCustomData(CustomData* customData);

const QScopedPointer<Ui::EditGroupWidgetMain> m_mainUi;

QPointer<QScrollArea> m_editGroupWidgetMain;
Expand Down
18 changes: 18 additions & 0 deletions src/gui/group/EditGroupWidgetBrowser.ui
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,23 @@
</widget>
</item>
<item row="5" column="0">
<widget class="QLabel" name="browserIntegrationRestrictKey">
<property name="text">
<string>Restrict matching to given browser key:</string>
</property>
<property name="alignment">
<set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set>
</property>
</widget>
</item>
<item row="5" column="1">
<widget class="QComboBox" name="browserIntegrationRestrictKeyCombobox">
<property name="accessibleName">
<string>Restrict matching to given browser key toggle for this and sub groups</string>
</property>
</widget>
</item>
<item row="6" column="0">
<spacer name="verticalSpacer_1">
<property name="orientation">
<enum>Qt::Vertical</enum>
Expand All @@ -158,6 +175,7 @@
<tabstop>browserIntegrationOnlyHttpAuthComboBox</tabstop>
<tabstop>browserIntegrationNotHttpAuthComboBox</tabstop>
<tabstop>browserIntegrationOmitWwwCombobox</tabstop>
<tabstop>browserIntegrationRestrictKeyCombobox</tabstop>
</tabstops>
<resources/>
<connections/>
Expand Down
65 changes: 65 additions & 0 deletions tests/TestBrowser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -757,3 +757,68 @@ void TestBrowser::testIsUrlIdentical()
QVERIFY(!browserService()->isUrlIdentical("https://example.com/", "://example.com/"));
QVERIFY(browserService()->isUrlIdentical("ftp://127.0.0.1/", "ftp://127.0.0.1"));
}

void TestBrowser::testRestrictBrowserKey()
{
auto db = QSharedPointer<Database>::create();
auto* root = db->rootGroup();

// Group 0 (root): No browser key restriction given
QStringList urlsRoot = {"https://example.com/0"};
auto entriesRoot = createEntries(urlsRoot, root);

// Group 1: restricted to browser with 'key1'
auto* group1 = new Group();
group1->setParent(root);
group1->setName("TestGroup1");
group1->customData()->set(BrowserService::OPTION_RESTRICT_KEY, "key1");
QStringList urls1 = {"https://example.com/1"};
auto entries1 = createEntries(urls1, group1);

// Group 2: restricted to browser with 'key2'
auto* group2 = new Group();
group2->setParent(root);
group2->setName("TestGroup2");
group2->customData()->set(BrowserService::OPTION_RESTRICT_KEY, "key2");
QStringList urls2 = {"https://example.com/2"};
auto entries2 = createEntries(urls2, group2);

// Group 2b: inherits parent group (2) restriction
auto* group2b = new Group();
group2b->setParent(group2);
group2b->setName("TestGroup2b");
QStringList urls2b = {"https://example.com/2b"};
auto entries2b = createEntries(urls2b, group2b);

// Group 3: inherits parent group (root) - any browser can see
auto* group3 = new Group();
group3->setParent(root);
group3->setName("TestGroup3");
QStringList urls3 = {"https://example.com/3"};
auto entries3 = createEntries(urls3, group3);

// Browser 'key0': Groups 1 and 2 are excluded, so entries 0 and 3 will be found
auto siteUrl = QString("https://example.com");
auto result = m_browserService->searchEntries(db, siteUrl, siteUrl, {"key0"});
auto sorted = m_browserService->sortEntries(result, siteUrl, siteUrl);
QCOMPARE(sorted.size(), 2);
QCOMPARE(sorted[0]->url(), QString("https://example.com/3"));
QCOMPARE(sorted[1]->url(), QString("https://example.com/0"));

// Browser 'key1': Group 2 will be excluded, so entries 0, 1, and 3 will be found
result = m_browserService->searchEntries(db, siteUrl, siteUrl, {"key1"});
sorted = m_browserService->sortEntries(result, siteUrl, siteUrl);
QCOMPARE(sorted.size(), 3);
QCOMPARE(sorted[0]->url(), QString("https://example.com/3"));
QCOMPARE(sorted[1]->url(), QString("https://example.com/1"));
QCOMPARE(sorted[2]->url(), QString("https://example.com/0"));

// Browser 'key2': Group 1 will be excluded, so entries 0, 2, 2b, 3 will be found
result = m_browserService->searchEntries(db, siteUrl, siteUrl, {"key2"});
sorted = m_browserService->sortEntries(result, siteUrl, siteUrl);
QCOMPARE(sorted.size(), 4);
QCOMPARE(sorted[0]->url(), QString("https://example.com/3"));
QCOMPARE(sorted[1]->url(), QString("https://example.com/2b"));
QCOMPARE(sorted[2]->url(), QString("https://example.com/2"));
QCOMPARE(sorted[3]->url(), QString("https://example.com/0"));
}
1 change: 1 addition & 0 deletions tests/TestBrowser.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ private slots:
void testBestMatchingCredentials();
void testBestMatchingWithAdditionalURLs();
void testIsUrlIdentical();
void testRestrictBrowserKey();

private:
QList<Entry*> createEntries(QStringList& urls, Group* root) const;
Expand Down

0 comments on commit 42fbf09

Please sign in to comment.