Fix various bugs when returning credentials (#9136)

Co-authored-by: Sami Vänttinen <sami.vanttinen@protonmail.com>
This commit is contained in:
Jonathan White 2024-01-11 07:16:09 -05:00
parent e401e8f4bc
commit c5312d63f2
3 changed files with 59 additions and 41 deletions

View File

@ -241,13 +241,16 @@ QJsonObject BrowserAction::handleGetLogins(const QJsonObject& json, const QStrin
entryParameters.hash = browserRequest.hash; entryParameters.hash = browserRequest.hash;
entryParameters.siteUrl = siteUrl; entryParameters.siteUrl = siteUrl;
entryParameters.formUrl = formUrl; entryParameters.formUrl = formUrl;
entryParameters.httpAuth = httpAuth;
const auto users = browserService()->findEntries(entryParameters, keyList, httpAuth); bool entriesFound = false;
if (users.isEmpty()) { const auto entries = browserService()->findEntries(entryParameters, keyList, &entriesFound);
if (!entriesFound) {
return getErrorReply(action, ERROR_KEEPASS_NO_LOGINS_FOUND); return getErrorReply(action, ERROR_KEEPASS_NO_LOGINS_FOUND);
} }
const Parameters params{{"count", users.count()}, {"entries", users}, {"hash", browserRequest.hash}, {"id", id}}; const Parameters params{
{"count", entries.count()}, {"entries", entries}, {"hash", browserRequest.hash}, {"id", id}};
return buildResponse(action, browserRequest.incrementedNonce, params); return buildResponse(action, browserRequest.incrementedNonce, params);
} }

View File

@ -1,6 +1,7 @@
/* /*
* Copyright (C) 2023 KeePassXC Team <team@keepassxc.org>
* Copyright (C) 2017 Sami Vänttinen <sami.vanttinen@protonmail.com>
* Copyright (C) 2013 Francois Ferrand * Copyright (C) 2013 Francois Ferrand
* Copyright (C) 2022 KeePassXC Team <team@keepassxc.org>
* *
* This program is free software: you can redistribute it and/or modify * 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 * it under the terms of the GNU General Public License as published by
@ -314,27 +315,31 @@ QString BrowserService::getCurrentTotp(const QString& uuid)
} }
QJsonArray QJsonArray
BrowserService::findEntries(const EntryParameters& entryParameters, const StringPairList& keyList, const bool httpAuth) BrowserService::findEntries(const EntryParameters& entryParameters, const StringPairList& keyList, bool* entriesFound)
{ {
if (entriesFound) {
*entriesFound = false;
}
const bool alwaysAllowAccess = browserSettings()->alwaysAllowAccess(); const bool alwaysAllowAccess = browserSettings()->alwaysAllowAccess();
const bool ignoreHttpAuth = browserSettings()->httpAuthPermission(); const bool ignoreHttpAuth = browserSettings()->httpAuthPermission();
const QString siteHost = QUrl(entryParameters.siteUrl).host(); const QString siteHost = QUrl(entryParameters.siteUrl).host();
const QString formHost = QUrl(entryParameters.formUrl).host(); const QString formHost = QUrl(entryParameters.formUrl).host();
// Check entries for authorization // Check entries for authorization
QList<Entry*> pwEntriesToConfirm; QList<Entry*> entriesToConfirm;
QList<Entry*> pwEntries; QList<Entry*> allowedEntries;
for (auto* entry : searchEntries(entryParameters.siteUrl, entryParameters.formUrl, keyList)) { for (auto* entry : searchEntries(entryParameters.siteUrl, entryParameters.formUrl, keyList)) {
auto entryCustomData = entry->customData(); auto entryCustomData = entry->customData();
if (!httpAuth if (!entryParameters.httpAuth
&& ((entryCustomData->contains(BrowserService::OPTION_ONLY_HTTP_AUTH) && ((entryCustomData->contains(BrowserService::OPTION_ONLY_HTTP_AUTH)
&& entryCustomData->value(BrowserService::OPTION_ONLY_HTTP_AUTH) == TRUE_STR) && entryCustomData->value(BrowserService::OPTION_ONLY_HTTP_AUTH) == TRUE_STR)
|| entry->group()->resolveCustomDataTriState(BrowserService::OPTION_ONLY_HTTP_AUTH) == Group::Enable)) { || entry->group()->resolveCustomDataTriState(BrowserService::OPTION_ONLY_HTTP_AUTH) == Group::Enable)) {
continue; continue;
} }
if (httpAuth if (entryParameters.httpAuth
&& ((entryCustomData->contains(BrowserService::OPTION_NOT_HTTP_AUTH) && ((entryCustomData->contains(BrowserService::OPTION_NOT_HTTP_AUTH)
&& entryCustomData->value(BrowserService::OPTION_NOT_HTTP_AUTH) == TRUE_STR) && entryCustomData->value(BrowserService::OPTION_NOT_HTTP_AUTH) == TRUE_STR)
|| entry->group()->resolveCustomDataTriState(BrowserService::OPTION_NOT_HTTP_AUTH) == Group::Enable)) { || entry->group()->resolveCustomDataTriState(BrowserService::OPTION_NOT_HTTP_AUTH) == Group::Enable)) {
@ -342,8 +347,8 @@ BrowserService::findEntries(const EntryParameters& entryParameters, const String
} }
// HTTP Basic Auth always needs a confirmation // HTTP Basic Auth always needs a confirmation
if (!ignoreHttpAuth && httpAuth) { if (!ignoreHttpAuth && entryParameters.httpAuth) {
pwEntriesToConfirm.append(entry); entriesToConfirm.append(entry);
continue; continue;
} }
@ -353,27 +358,27 @@ BrowserService::findEntries(const EntryParameters& entryParameters, const String
case Unknown: case Unknown:
if (alwaysAllowAccess) { if (alwaysAllowAccess) {
pwEntries.append(entry); allowedEntries.append(entry);
} else { } else {
pwEntriesToConfirm.append(entry); entriesToConfirm.append(entry);
} }
break; break;
case Allowed: case Allowed:
pwEntries.append(entry); allowedEntries.append(entry);
break; break;
} }
} }
// Confirm entries if (entriesToConfirm.isEmpty() && allowedEntries.isEmpty()) {
QList<Entry*> selectedEntriesToConfirm = return {};
confirmEntries(pwEntriesToConfirm, entryParameters, siteHost, formHost, httpAuth);
if (!selectedEntriesToConfirm.isEmpty()) {
pwEntries.append(selectedEntriesToConfirm);
} }
if (pwEntries.isEmpty()) { // Confirm entries
return {}; auto selectedEntriesToConfirm =
confirmEntries(entriesToConfirm, entryParameters, siteHost, formHost, entryParameters.httpAuth);
if (!selectedEntriesToConfirm.isEmpty()) {
allowedEntries.append(selectedEntriesToConfirm);
} }
// Ensure that database is not locked when the popup was visible // Ensure that database is not locked when the popup was visible
@ -382,24 +387,28 @@ BrowserService::findEntries(const EntryParameters& entryParameters, const String
} }
// Sort results // Sort results
pwEntries = sortEntries(pwEntries, entryParameters.siteUrl, entryParameters.formUrl); allowedEntries = sortEntries(allowedEntries, entryParameters.siteUrl, entryParameters.formUrl);
// Fill the list // Fill the list
QJsonArray result; QJsonArray entries;
for (auto* entry : pwEntries) { for (auto* entry : allowedEntries) {
result.append(prepareEntry(entry)); entries.append(prepareEntry(entry));
} }
return result; if (entriesFound != nullptr) {
*entriesFound = true;
}
return entries;
} }
QList<Entry*> BrowserService::confirmEntries(QList<Entry*>& pwEntriesToConfirm, QList<Entry*> BrowserService::confirmEntries(QList<Entry*>& entriesToConfirm,
const EntryParameters& entryParameters, const EntryParameters& entryParameters,
const QString& siteHost, const QString& siteHost,
const QString& formUrl, const QString& formUrl,
const bool httpAuth) const bool httpAuth)
{ {
if (pwEntriesToConfirm.isEmpty() || m_dialogActive) { if (entriesToConfirm.isEmpty() || m_dialogActive) {
return {}; return {};
} }
@ -410,20 +419,25 @@ QList<Entry*> BrowserService::confirmEntries(QList<Entry*>& pwEntriesToConfirm,
connect(m_currentDatabaseWidget, SIGNAL(databaseLockRequested()), &accessControlDialog, SLOT(reject())); connect(m_currentDatabaseWidget, SIGNAL(databaseLockRequested()), &accessControlDialog, SLOT(reject()));
connect(&accessControlDialog, &BrowserAccessControlDialog::disableAccess, [&](QTableWidgetItem* item) { connect(&accessControlDialog, &BrowserAccessControlDialog::disableAccess, [&](QTableWidgetItem* item) {
auto entry = pwEntriesToConfirm[item->row()]; auto entry = entriesToConfirm[item->row()];
denyEntry(entry, siteHost, formUrl, entryParameters.realm); denyEntry(entry, siteHost, formUrl, entryParameters.realm);
}); });
accessControlDialog.setItems(pwEntriesToConfirm, entryParameters.siteUrl, httpAuth); accessControlDialog.setItems(entriesToConfirm, entryParameters.siteUrl, httpAuth);
QList<Entry*> allowedEntries; QList<Entry*> allowedEntries;
auto ret = accessControlDialog.exec(); auto ret = accessControlDialog.exec();
if (ret == QDialog::Accepted) {
for (auto item : accessControlDialog.getSelectedEntries()) { for (auto item : accessControlDialog.getSelectedEntries()) {
auto entry = pwEntriesToConfirm[item->row()]; auto entry = entriesToConfirm[item->row()];
if (accessControlDialog.remember()) { if (accessControlDialog.remember()) {
if (ret == QDialog::Accepted) {
allowEntry(entry, siteHost, formUrl, entryParameters.realm); allowEntry(entry, siteHost, formUrl, entryParameters.realm);
} else {
denyEntry(entry, siteHost, formUrl, entryParameters.realm);
} }
}
if (ret == QDialog::Accepted) {
allowedEntries.append(entry); allowedEntries.append(entry);
} }
} }
@ -866,11 +880,11 @@ void BrowserService::requestGlobalAutoType(const QString& search)
emit osUtils->globalShortcutTriggered("autotype", search); emit osUtils->globalShortcutTriggered("autotype", search);
} }
QList<Entry*> BrowserService::sortEntries(QList<Entry*>& pwEntries, const QString& siteUrl, const QString& formUrl) QList<Entry*> BrowserService::sortEntries(QList<Entry*>& entries, const QString& siteUrl, const QString& formUrl)
{ {
// Build map of prioritized entries // Build map of prioritized entries
QMultiMap<int, Entry*> priorities; QMultiMap<int, Entry*> priorities;
for (auto* entry : pwEntries) { for (auto* entry : entries) {
priorities.insert(sortPriority(getEntryURLs(entry), siteUrl, formUrl), entry); priorities.insert(sortPriority(getEntryURLs(entry), siteUrl, formUrl), entry);
} }

View File

@ -1,6 +1,7 @@
/* /*
* Copyright (C) 2023 KeePassXC Team <team@keepassxc.org>
* Copyright (C) 2017 Sami Vänttinen <sami.vanttinen@protonmail.com>
* Copyright (C) 2013 Francois Ferrand * Copyright (C) 2013 Francois Ferrand
* Copyright (C) 2022 KeePassXC Team <team@keepassxc.org>
* *
* This program is free software: you can redistribute it and/or modify * 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 * it under the terms of the GNU General Public License as published by
@ -50,6 +51,7 @@ struct EntryParameters
QString hash; QString hash;
QString siteUrl; QString siteUrl;
QString formUrl; QString formUrl;
bool httpAuth;
}; };
class DatabaseWidget; class DatabaseWidget;
@ -88,8 +90,7 @@ public:
const QSharedPointer<Database>& selectedDb = {}); const QSharedPointer<Database>& selectedDb = {});
bool updateEntry(const EntryParameters& entryParameters, const QString& uuid); bool updateEntry(const EntryParameters& entryParameters, const QString& uuid);
bool deleteEntry(const QString& uuid); bool deleteEntry(const QString& uuid);
QJsonArray QJsonArray findEntries(const EntryParameters& entryParameters, const StringPairList& keyList, bool* entriesFound);
findEntries(const EntryParameters& entryParameters, const StringPairList& keyList, const bool httpAuth = false);
void requestGlobalAutoType(const QString& search); void requestGlobalAutoType(const QString& search);
static void convertAttributesToCustomData(QSharedPointer<Database> db); static void convertAttributesToCustomData(QSharedPointer<Database> db);
@ -131,8 +132,8 @@ private:
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);
QList<Entry*> searchEntries(const QString& siteUrl, const QString& formUrl, const StringPairList& keyList); QList<Entry*> searchEntries(const QString& siteUrl, const QString& formUrl, const StringPairList& keyList);
QList<Entry*> sortEntries(QList<Entry*>& pwEntries, const QString& siteUrl, const QString& formUrl); QList<Entry*> sortEntries(QList<Entry*>& entries, const QString& siteUrl, const QString& formUrl);
QList<Entry*> confirmEntries(QList<Entry*>& pwEntriesToConfirm, QList<Entry*> confirmEntries(QList<Entry*>& entriesToConfirm,
const EntryParameters& entryParameters, const EntryParameters& entryParameters,
const QString& siteHost, const QString& siteHost,
const QString& formUrl, const QString& formUrl,