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 Oct 31, 2023
1 parent 6f2354c commit 3bf086e
Show file tree
Hide file tree
Showing 10 changed files with 207 additions and 6 deletions.
12 changes: 12 additions & 0 deletions share/translations/keepassxc_en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,10 @@ Do you want to delete the entry?
</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Disable</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>%1 (Passkey)</source>
<translation type="unfinished"></translation>
Expand Down Expand Up @@ -3184,6 +3188,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
33 changes: 27 additions & 6 deletions src/browser/BrowserService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,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 @@ -927,6 +928,7 @@ bool BrowserService::deleteEntry(const QString& uuid)
QList<Entry*> BrowserService::searchEntries(const QSharedPointer<Database>& db,
const QString& siteUrl,
const QString& formUrl,
const QList<QString>& keys,
bool passkey)
{
QList<Entry*> entries;
Expand All @@ -941,6 +943,12 @@ QList<Entry*> BrowserService::searchEntries(const QSharedPointer<Database>& db,
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 @@ -977,30 +985,35 @@ QList<Entry*> BrowserService::searchEntries(const QString& siteUrl,
const StringPairList& keyList,
bool passkey)
{
// 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 @@ -1009,13 +1022,21 @@ QList<Entry*> BrowserService::searchEntries(const QString& siteUrl,
QList<Entry*> entries;
do {
for (const auto& db : databases) {
entries << searchEntries(db, siteUrl, formUrl, passkey);
entries << searchEntries(db, siteUrl, formUrl, keys, passkey);
}
} 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
4 changes: 4 additions & 0 deletions src/browser/BrowserService.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,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 @@ -157,6 +160,7 @@ private slots:
QList<Entry*> searchEntries(const QSharedPointer<Database>& db,
const QString& siteUrl,
const QString& formUrl,
const QList<QString>& keys = QList<QString>(),
bool passkey = false);
QList<Entry*>
searchEntries(const QString& siteUrl, const QString& formUrl, const StringPairList& keyList, bool passkey = false);
Expand Down
15 changes: 15 additions & 0 deletions src/core/Group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,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 @@ -105,6 +105,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
60 changes: 60 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,58 @@ Group::TriState EditGroupWidget::triStateFromIndex(int index)
return Group::Inherit;
}
}

#ifdef WITH_XC_BROWSER
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());
}
}
#endif
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 @@ -673,3 +673,68 @@ void TestBrowser::testBestMatchingWithAdditionalURLs()
QCOMPARE(sorted.length(), 1);
QCOMPARE(sorted[0]->url(), urls[0]);
}

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 @@ -49,6 +49,7 @@ private slots:
void testSubdomainsAndPaths();
void testBestMatchingCredentials();
void testBestMatchingWithAdditionalURLs();
void testRestrictBrowserKey();

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

0 comments on commit 3bf086e

Please sign in to comment.