Skip to content

Commit

Permalink
Improve Auto-Type Select Dialog
Browse files Browse the repository at this point in the history 
Significant improvements to the Auto-Type select dialog. Reduce stale and unnecessary code paths.

* Close select dialog when databases are locked.
* Close open modal dialogs prior to showing the Auto-Type select dialog to prevent interference.
* Never perform Auto-Type on the KeePassXC window.
* Only filter match list based on Group, Title, and Username column data (ie, ignore sequence column)
* Always show the sequence column (revert feature)
* Show selection dialog if there are no matches to allow for a database search

* Close #3630 - Allow typing {USERNAME} and {PASSWORD} from selection dialog (right-click menu).
* Close #429 - Ability to search open databases for an entry from the Auto-Type selection dialog.
* Fix #5361 - Default size of selection dialog doesn't cut off matches
  • Loading branch information
@droidmonkey
droidmonkey committed Dec 17, 2020
1 parent 92f627b commit 0c9c841
Show file tree
Hide file tree
Showing 32 changed files with 749 additions and 1,013 deletions.
8 changes: 2 additions & 6 deletions src/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ endif(NOT ZXCVBN_LIBRARIES)
set(keepassx_SOURCES
core/Alloc.cpp
core/AutoTypeAssociations.cpp
core/AutoTypeMatch.cpp
core/Base32.cpp
core/Bootstrap.cpp
core/Clock.cpp
Expand Down Expand Up @@ -139,8 +138,6 @@ set(keepassx_SOURCES
gui/csvImport/CsvImportWizard.cpp
gui/csvImport/CsvParserModel.cpp
gui/entry/AutoTypeAssociationsModel.cpp
gui/entry/AutoTypeMatchModel.cpp
gui/entry/AutoTypeMatchView.cpp
gui/entry/EditEntryWidget.cpp
gui/entry/EntryAttachmentsModel.cpp
gui/entry/EntryAttachmentsWidget.cpp
Expand Down Expand Up @@ -273,11 +270,10 @@ set(autotype_SOURCES
core/Tools.cpp
autotype/AutoType.cpp
autotype/AutoTypeAction.cpp
autotype/AutoTypeFilterLineEdit.cpp
autotype/AutoTypeMatchModel.cpp
autotype/AutoTypeMatchView.cpp
autotype/AutoTypeSelectDialog.cpp
autotype/AutoTypeSelectView.cpp
autotype/ShortcutWidget.cpp
autotype/WildcardMatcher.cpp
autotype/WindowSelectComboBox.cpp)

if(MINGW)
Expand Down
204 changes: 49 additions & 155 deletions src/autotype/AutoType.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@
#include <QApplication>
#include <QPluginLoader>
#include <QRegularExpression>
#include <QWindow>

#include "config-keepassx.h"

#include "autotype/AutoTypePlatformPlugin.h"
#include "autotype/AutoTypeSelectDialog.h"
#include "autotype/WildcardMatcher.h"
#include "core/AutoTypeMatch.h"
#include "core/Config.h"
#include "core/Database.h"
#include "core/Entry.h"
Expand Down Expand Up @@ -250,12 +249,10 @@ void AutoType::performAutoType(const Entry* entry, QWidget* hideWindow)
return;
}

QList<QString> sequences = autoTypeSequences(entry);
if (sequences.isEmpty()) {
return;
auto sequences = entry->autoTypeSequences();
if (!sequences.isEmpty()) {
executeAutoTypeActions(entry, hideWindow, sequences.first());
}

executeAutoTypeActions(entry, hideWindow, sequences.first());
}

/**
Expand All @@ -273,6 +270,11 @@ void AutoType::performAutoTypeWithSequence(const Entry* entry, const QString& se

void AutoType::startGlobalAutoType()
{
// Never Auto-Type into KeePassXC itself
if (qApp->focusWindow()) {
return;
}

m_windowForGlobal = m_plugin->activeWindow();
m_windowTitleForGlobal = m_plugin->activeWindowTitle();
#ifdef Q_OS_MACOS
Expand Down Expand Up @@ -331,58 +333,62 @@ void AutoType::performGlobalAutoType(const QList<QSharedPointer<Database>>& dbLi

for (const auto& db : dbList) {
const QList<Entry*> dbEntries = db->rootGroup()->entriesRecursive();
for (Entry* entry : dbEntries) {
for (auto entry : dbEntries) {
auto group = entry->group();
if (!group || !group->resolveAutoTypeEnabled() || !entry->autoTypeEnabled()) {
continue;
}

if (hideExpired && entry->isExpired()) {
continue;
}
const QSet<QString> sequences = autoTypeSequences(entry, m_windowTitleForGlobal).toSet();
for (const QString& sequence : sequences) {
if (!sequence.isEmpty()) {
matchList << AutoTypeMatch(entry, sequence);
}
auto sequences = entry->autoTypeSequences(m_windowTitleForGlobal).toSet();
for (const auto& sequence : sequences) {
matchList << AutoTypeMatch(entry, sequence);
}
}
}

if (matchList.isEmpty()) {
if (qobject_cast<QApplication*>(QCoreApplication::instance())) {
auto* msgBox = new QMessageBox();
msgBox->setAttribute(Qt::WA_DeleteOnClose);
msgBox->setWindowTitle(tr("Auto-Type - KeePassXC"));
msgBox->setText(tr("Couldn't find an entry that matches the window title:")
.append("\n\n")
.append(m_windowTitleForGlobal));
msgBox->setIcon(QMessageBox::Information);
msgBox->setStandardButtons(QMessageBox::Ok);
#ifdef Q_OS_MACOS
m_plugin->raiseOwnWindow();
Tools::wait(200);
#endif
msgBox->exec();
restoreWindowState();
// Show the selection dialog if we always ask, have multiple matches, or no matches
if (config()->get(Config::Security_AutoTypeAsk).toBool() || matchList.size() > 1 || matchList.isEmpty()) {
// Close any open modal windows that would interfere with the process
if (qApp->modalWindow()) {
qApp->modalWindow()->close();
}

m_inGlobalAutoTypeDialog.unlock();
emit autotypeRejected();
} else if ((matchList.size() == 1) && !config()->get(Config::Security_AutoTypeAsk).toBool()) {
executeAutoTypeActions(matchList.first().entry, nullptr, matchList.first().sequence, m_windowForGlobal);
m_inGlobalAutoTypeDialog.unlock();
} else {
auto* selectDialog = new AutoTypeSelectDialog();
selectDialog->setMatches(matchList, dbList);

// connect slots, both of which must unlock the m_inGlobalAutoTypeDialog mutex
connect(selectDialog, SIGNAL(matchActivated(AutoTypeMatch)), SLOT(performAutoTypeFromGlobal(AutoTypeMatch)));
connect(selectDialog, SIGNAL(rejected()), SLOT(autoTypeRejectedFromGlobal()));
connect(getMainWindow(), &MainWindow::databaseLocked, selectDialog, &AutoTypeSelectDialog::reject);
connect(selectDialog, &AutoTypeSelectDialog::matchActivated, this, [this](AutoTypeMatch match) {
restoreWindowState();
QApplication::processEvents();
m_plugin->raiseWindow(m_windowForGlobal);
executeAutoTypeActions(match.first, nullptr, match.second, m_windowForGlobal);
resetAutoTypeState();
});
connect(selectDialog, &QDialog::rejected, this, [this] {
restoreWindowState();
resetAutoTypeState();
emit autotypeRejected();
});

selectDialog->setMatchList(matchList);
#ifdef Q_OS_MACOS
m_plugin->raiseOwnWindow();
Tools::wait(200);
#endif
selectDialog->show();
selectDialog->raise();
// necessary when the main window is minimized
selectDialog->activateWindow();
} else if (!matchList.isEmpty()) {
// Only one match and not asking, do it!
executeAutoTypeActions(matchList.first().first, nullptr, matchList.first().second, m_windowForGlobal);
resetAutoTypeState();
} else {
// We should never get here
Q_ASSERT(false);
resetAutoTypeState();
emit autotypeRejected();
}
}

Expand All @@ -399,29 +405,12 @@ void AutoType::restoreWindowState()
#endif
}

void AutoType::performAutoTypeFromGlobal(AutoTypeMatch match)
{
restoreWindowState();

m_plugin->raiseWindow(m_windowForGlobal);
executeAutoTypeActions(match.entry, nullptr, match.sequence, m_windowForGlobal);

// make sure the mutex is definitely locked before we unlock it
Q_UNUSED(m_inGlobalAutoTypeDialog.tryLock());
m_inGlobalAutoTypeDialog.unlock();
}

void AutoType::autoTypeRejectedFromGlobal()
void AutoType::resetAutoTypeState()
{
// this slot can be called twice when the selection dialog is deleted,
// so make sure the mutex is locked before we try unlocking it
Q_UNUSED(m_inGlobalAutoTypeDialog.tryLock());
m_inGlobalAutoTypeDialog.unlock();
m_windowForGlobal = 0;
m_windowTitleForGlobal.clear();

restoreWindowState();
emit autotypeRejected();
Q_UNUSED(m_inGlobalAutoTypeDialog.tryLock());
m_inGlobalAutoTypeDialog.unlock();
}

/**
Expand Down Expand Up @@ -622,101 +611,6 @@ QList<AutoTypeAction*> AutoType::createActionFromTemplate(const QString& tmpl, c
return list;
}

/**
* Retrive the autotype sequences matches for a given windowTitle
* This returns a list with priority ordering. If you don't want duplicates call .toSet() on it.
*/
QList<QString> AutoType::autoTypeSequences(const Entry* entry, const QString& windowTitle)
{
QList<QString> sequenceList;
const Group* group = entry->group();

if (!group || !entry->autoTypeEnabled()) {
return sequenceList;
}

do {
if (group->autoTypeEnabled() == Group::Disable) {
return sequenceList;
} else if (group->autoTypeEnabled() == Group::Enable) {
break;
}
group = group->parentGroup();

} while (group);

if (!windowTitle.isEmpty()) {
const QList<AutoTypeAssociations::Association> assocList = entry->autoTypeAssociations()->getAll();
for (const AutoTypeAssociations::Association& assoc : assocList) {
const QString window = entry->resolveMultiplePlaceholders(assoc.window);
if (windowMatches(windowTitle, window)) {
if (!assoc.sequence.isEmpty()) {
sequenceList.append(assoc.sequence);
} else {
sequenceList.append(entry->effectiveAutoTypeSequence());
}
}
}

if (config()->get(Config::AutoTypeEntryTitleMatch).toBool()
&& windowMatchesTitle(windowTitle, entry->resolvePlaceholder(entry->title()))) {
sequenceList.append(entry->effectiveAutoTypeSequence());
}

if (config()->get(Config::AutoTypeEntryURLMatch).toBool()
&& windowMatchesUrl(windowTitle, entry->resolvePlaceholder(entry->url()))) {
sequenceList.append(entry->effectiveAutoTypeSequence());
}

if (sequenceList.isEmpty()) {
return sequenceList;
}
} else {
sequenceList.append(entry->effectiveAutoTypeSequence());
}

return sequenceList;
}

/**
* Checks if a window title matches a pattern
*/
bool AutoType::windowMatches(const QString& windowTitle, const QString& windowPattern)
{
if (windowPattern.startsWith("//") && windowPattern.endsWith("//") && windowPattern.size() >= 4) {
QRegExp regExp(windowPattern.mid(2, windowPattern.size() - 4), Qt::CaseInsensitive, QRegExp::RegExp2);
return (regExp.indexIn(windowTitle) != -1);
}
return WildcardMatcher(windowTitle).match(windowPattern);
}

/**
* Checks if a window title matches an entry Title
* The entry title should be Spr-compiled by the caller
*/
bool AutoType::windowMatchesTitle(const QString& windowTitle, const QString& resolvedTitle)
{
return !resolvedTitle.isEmpty() && windowTitle.contains(resolvedTitle, Qt::CaseInsensitive);
}

/**
* Checks if a window title matches an entry URL
* The entry URL should be Spr-compiled by the caller
*/
bool AutoType::windowMatchesUrl(const QString& windowTitle, const QString& resolvedUrl)
{
if (!resolvedUrl.isEmpty() && windowTitle.contains(resolvedUrl, Qt::CaseInsensitive)) {
return true;
}

QUrl url(resolvedUrl);
if (url.isValid() && !url.host().isEmpty()) {
return windowTitle.contains(url.host(), Qt::CaseInsensitive);
}

return false;
}

/**
* Checks if the overall syntax of an autotype sequence is fine
*/
Expand Down
9 changes: 2 additions & 7 deletions src/autotype/AutoType.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#include <QStringList>
#include <QWidget>

#include "core/AutoTypeMatch.h"
#include "autotype/AutoTypeMatch.h"

class AutoTypeAction;
class AutoTypeExecutor;
Expand Down Expand Up @@ -68,8 +68,6 @@ public slots:

private slots:
void startGlobalAutoType();
void performAutoTypeFromGlobal(AutoTypeMatch match);
void autoTypeRejectedFromGlobal();
void unloadPlugin();

private:
Expand All @@ -89,11 +87,8 @@ private slots:
WId window = 0);
bool parseActions(const QString& sequence, const Entry* entry, QList<AutoTypeAction*>& actions);
QList<AutoTypeAction*> createActionFromTemplate(const QString& tmpl, const Entry* entry);
QList<QString> autoTypeSequences(const Entry* entry, const QString& windowTitle = QString());
bool windowMatchesTitle(const QString& windowTitle, const QString& resolvedTitle);
bool windowMatchesUrl(const QString& windowTitle, const QString& resolvedUrl);
bool windowMatches(const QString& windowTitle, const QString& windowPattern);
void restoreWindowState();
void resetAutoTypeState();

QMutex m_inAutoType;
QMutex m_inGlobalAutoTypeDialog;
Expand Down
39 changes: 0 additions & 39 deletions src/autotype/AutoTypeFilterLineEdit.cpp
View file

This file was deleted.

Loading

0 comments on commit 0c9c841

Please sign in to comment.