Skip to content

Commit

Permalink
Fix global Auto-Type when database locked
Browse files Browse the repository at this point in the history 
* Store the currently active window right when the global keyboard shortcut is triggered
* Eliminate unnecessary window raise/lower and delays on macOS
* Remove duplicate addition of macutils symbols from mac Auto-Type plugin
* Fix tests to fake trigger a global autotype sequence
  • Loading branch information
@droidmonkey
droidmonkey committed May 12, 2019
1 parent fed8a56 commit cdc51f3
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 24 deletions.
40 changes: 26 additions & 14 deletions src/autotype/AutoType.cpp
View file
Expand Up @@ -47,7 +47,7 @@ AutoType::AutoType(QObject* parent, bool test)
, m_pluginLoader(new QPluginLoader(this))
, m_plugin(nullptr)
, m_executor(nullptr)
, m_windowFromGlobal(0)
, m_windowForGlobal(0)
{
// prevent crash when the plugin has unresolved symbols
m_pluginLoader->setLoadHints(QLibrary::ResolveAllSymbolsHint);
Expand Down Expand Up @@ -90,7 +90,7 @@ void AutoType::loadPlugin(const QString& pluginPath)
if (m_plugin) {
if (m_plugin->isAvailable()) {
m_executor = m_plugin->createExecutor();
connect(pluginInstance, SIGNAL(globalShortcutTriggered()), SIGNAL(globalShortcutTriggered()));
connect(pluginInstance, SIGNAL(globalShortcutTriggered()), SLOT(startGlobalAutoType()));
} else {
unloadPlugin();
}
Expand Down Expand Up @@ -222,6 +222,7 @@ void AutoType::executeAutoTypeActions(const Entry* entry, QWidget* hideWindow, c

Tools::wait(qMax(100, config()->get("AutoTypeStartDelay", 500).toInt()));

// Used only for selected entry auto-type
if (!window) {
window = m_plugin->activeWindow();
}
Expand All @@ -240,6 +241,9 @@ void AutoType::executeAutoTypeActions(const Entry* entry, QWidget* hideWindow, c
QCoreApplication::processEvents(QEventLoop::AllEvents, 10);
}

m_windowForGlobal = 0;
m_windowTitleForGlobal.clear();

// emit signal only if autotype performed correctly
emit autotypePerformed();

Expand All @@ -264,6 +268,13 @@ void AutoType::performAutoType(const Entry* entry, QWidget* hideWindow)
executeAutoTypeActions(entry, hideWindow, sequences.first());
}

void AutoType::startGlobalAutoType()
{
m_windowForGlobal = m_plugin->activeWindow();
m_windowTitleForGlobal = m_plugin->activeWindowTitle();
emit globalAutoTypeTriggered();
}

/**
* Global Autotype entry-point function
* Perform global Auto-Type on the active window
Expand All @@ -278,9 +289,7 @@ void AutoType::performGlobalAutoType(const QList<QSharedPointer<Database>>& dbLi
return;
}

QString windowTitle = m_plugin->activeWindowTitle();

if (windowTitle.isEmpty()) {
if (m_windowTitleForGlobal.isEmpty()) {
m_inGlobalAutoTypeDialog.unlock();
return;
}
Expand All @@ -290,7 +299,7 @@ void AutoType::performGlobalAutoType(const QList<QSharedPointer<Database>>& dbLi
for (const auto& db : dbList) {
const QList<Entry*> dbEntries = db->rootGroup()->entriesRecursive();
for (Entry* entry : dbEntries) {
const QSet<QString> sequences = autoTypeSequences(entry, windowTitle).toSet();
const QSet<QString> sequences = autoTypeSequences(entry, m_windowTitleForGlobal).toSet();
for (const QString& sequence : sequences) {
if (!sequence.isEmpty()) {
matchList << AutoTypeMatch(entry, sequence);
Expand All @@ -304,8 +313,9 @@ void AutoType::performGlobalAutoType(const QList<QSharedPointer<Database>>& dbLi
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(windowTitle));
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);
msgBox->show();
Expand All @@ -316,31 +326,31 @@ void AutoType::performGlobalAutoType(const QList<QSharedPointer<Database>>& dbLi
m_inGlobalAutoTypeDialog.unlock();
emit autotypeRejected();
} else if ((matchList.size() == 1) && !config()->get("security/autotypeask").toBool()) {
executeAutoTypeActions(matchList.first().entry, nullptr, matchList.first().sequence);
executeAutoTypeActions(matchList.first().entry, nullptr, matchList.first().sequence, m_windowForGlobal);
m_inGlobalAutoTypeDialog.unlock();
} else {
m_windowFromGlobal = m_plugin->activeWindow();
auto* selectDialog = new AutoTypeSelectDialog();

// 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()));

selectDialog->setMatchList(matchList);
#if defined(Q_OS_MACOS)
#ifdef Q_OS_MACOS
m_plugin->raiseOwnWindow();
Tools::wait(500);
Tools::wait(200);
#endif
selectDialog->show();
selectDialog->raise();
// necessary when the main window is minimized
selectDialog->activateWindow();
}
}

void AutoType::performAutoTypeFromGlobal(AutoTypeMatch match)
{
m_plugin->raiseWindow(m_windowFromGlobal);
executeAutoTypeActions(match.entry, nullptr, match.sequence, m_windowFromGlobal);
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());
Expand All @@ -353,6 +363,8 @@ void AutoType::autoTypeRejectedFromGlobal()
// 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();

emit autotypeRejected();
}
Expand Down
9 changes: 6 additions & 3 deletions src/autotype/AutoType.h
View file
Expand Up @@ -62,18 +62,19 @@ public slots:
void raiseWindow();

signals:
void globalShortcutTriggered();
void globalAutoTypeTriggered();
void autotypePerformed();
void autotypeRejected();

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

private:
explicit AutoType(QObject* parent = nullptr, bool test = false);
~AutoType();
~AutoType() override;
void loadPlugin(const QString& pluginPath);
void executeAutoTypeActions(const Entry* entry,
QWidget* hideWindow = nullptr,
Expand All @@ -94,9 +95,11 @@ private slots:
QPluginLoader* m_pluginLoader;
AutoTypePlatformInterface* m_plugin;
AutoTypeExecutor* m_executor;
WId m_windowFromGlobal;
static AutoType* m_instance;

QString m_windowTitleForGlobal;
WId m_windowForGlobal;

Q_DISABLE_COPY(AutoType)
};

Expand Down
6 changes: 1 addition & 5 deletions src/autotype/mac/CMakeLists.txt
View file
@@ -1,10 +1,6 @@
set(autotype_mac_SOURCES AutoTypeMac.cpp)

set(autotype_mac_mm_SOURCES
${CMAKE_SOURCE_DIR}/src/gui/macutils/AppKitImpl.mm
${CMAKE_SOURCE_DIR}/src/gui/macutils/MacUtils.cpp)

add_library(keepassx-autotype-cocoa MODULE ${autotype_mac_SOURCES} ${autotype_mac_mm_SOURCES})
add_library(keepassx-autotype-cocoa MODULE ${autotype_mac_SOURCES})
set_target_properties(keepassx-autotype-cocoa PROPERTIES LINK_FLAGS "-framework Foundation -framework AppKit -framework Carbon")
target_link_libraries(keepassx-autotype-cocoa ${PROGNAME} Qt5::Core Qt5::Widgets)

Expand Down
5 changes: 5 additions & 0 deletions src/autotype/test/AutoTypeTest.cpp
View file
Expand Up @@ -68,6 +68,11 @@ AutoTypeExecutor* AutoTypePlatformTest::createExecutor()
return new AutoTypeExecutorTest(this);
}

void AutoTypePlatformTest::triggerGlobalAutoType()
{
emit globalShortcutTriggered();
}

void AutoTypePlatformTest::setActiveWindowTitle(const QString& title)
{
m_activeWindowTitle = title;
Expand Down
1 change: 1 addition & 0 deletions src/autotype/test/AutoTypeTest.h
View file
Expand Up @@ -48,6 +48,7 @@ class AutoTypePlatformTest : public QObject, public AutoTypePlatformInterface, p
bool raiseOwnWindow() override;
#endif

void triggerGlobalAutoType() override;
void setActiveWindowTitle(const QString& title) override;

QString actionChars() override;
Expand Down
1 change: 1 addition & 0 deletions src/autotype/test/AutoTypeTestInterface.h
View file
Expand Up @@ -26,6 +26,7 @@ class AutoTypeTestInterface
virtual ~AutoTypeTestInterface()
{
}
virtual void triggerGlobalAutoType() = 0;
virtual void setActiveWindowTitle(const QString& title) = 0;

virtual QString actionChars() = 0;
Expand Down
4 changes: 2 additions & 2 deletions src/gui/DatabaseTabWidget.cpp
View file
Expand Up @@ -59,7 +59,7 @@ DatabaseTabWidget::DatabaseTabWidget(QWidget* parent)
connect(this, SIGNAL(currentChanged(int)), SLOT(emitActivateDatabaseChanged()));
connect(this, SIGNAL(activateDatabaseChanged(DatabaseWidget*)),
m_dbWidgetStateSync, SLOT(setActive(DatabaseWidget*)));
connect(autoType(), SIGNAL(globalShortcutTriggered()), SLOT(performGlobalAutoType()));
connect(autoType(), SIGNAL(globalAutoTypeTriggered()), SLOT(performGlobalAutoType()));
connect(autoType(), SIGNAL(autotypePerformed()), SLOT(relockPendingDatabase()));
connect(autoType(), SIGNAL(autotypeRejected()), SLOT(relockPendingDatabase()));
// clang-format on
Expand Down Expand Up @@ -562,7 +562,7 @@ void DatabaseTabWidget::unlockDatabaseInDialog(DatabaseWidget* dbWidget,
#ifdef Q_OS_MACOS
if (intent == DatabaseOpenDialog::Intent::AutoType || intent == DatabaseOpenDialog::Intent::Browser) {
macUtils()->raiseOwnWindow();
Tools::wait(500);
Tools::wait(200);
}
#endif

Expand Down
15 changes: 15 additions & 0 deletions tests/TestAutoType.cpp
View file
Expand Up @@ -157,6 +157,7 @@ void TestAutoType::testGlobalAutoTypeWithNoMatch()
void TestAutoType::testGlobalAutoTypeWithOneMatch()
{
m_test->setActiveWindowTitle("custom window");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);

QCOMPARE(m_test->actionChars(), QString("%1association%2").arg(m_entry1->username()).arg(m_entry1->password()));
Expand All @@ -167,6 +168,7 @@ void TestAutoType::testGlobalAutoTypeTitleMatch()
config()->set("AutoTypeEntryTitleMatch", true);

m_test->setActiveWindowTitle("An Entry Title!");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);

QCOMPARE(m_test->actionChars(), QString("%1%2").arg(m_entry2->password(), m_test->keyToString(Qt::Key_Enter)));
Expand All @@ -177,6 +179,7 @@ void TestAutoType::testGlobalAutoTypeUrlMatch()
config()->set("AutoTypeEntryTitleMatch", true);

m_test->setActiveWindowTitle("Dummy - http://example.org/ - <My Browser>");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);

QCOMPARE(m_test->actionChars(), QString("%1%2").arg(m_entry5->password(), m_test->keyToString(Qt::Key_Enter)));
Expand All @@ -187,6 +190,7 @@ void TestAutoType::testGlobalAutoTypeUrlSubdomainMatch()
config()->set("AutoTypeEntryTitleMatch", true);

m_test->setActiveWindowTitle("Dummy - http://sub.example.org/ - <My Browser>");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);

QCOMPARE(m_test->actionChars(), QString("%1%2").arg(m_entry5->password(), m_test->keyToString(Qt::Key_Enter)));
Expand All @@ -195,6 +199,7 @@ void TestAutoType::testGlobalAutoTypeUrlSubdomainMatch()
void TestAutoType::testGlobalAutoTypeTitleMatchDisabled()
{
m_test->setActiveWindowTitle("An Entry Title!");
m_test->triggerGlobalAutoType();
MessageBox::setNextAnswer(MessageBox::Ok);
m_autoType->performGlobalAutoType(m_dbList);

Expand All @@ -205,58 +210,68 @@ void TestAutoType::testGlobalAutoTypeRegExp()
{
// substring matches are ok
m_test->setActiveWindowTitle("lorem REGEX1 ipsum");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);
QCOMPARE(m_test->actionChars(), QString("regex1"));
m_test->clearActions();

// should be case-insensitive
m_test->setActiveWindowTitle("lorem regex1 ipsum");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);
QCOMPARE(m_test->actionChars(), QString("regex1"));
m_test->clearActions();

// exact match
m_test->setActiveWindowTitle("REGEX2");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);
QCOMPARE(m_test->actionChars(), QString("regex2"));
m_test->clearActions();

// a bit more complicated regex
m_test->setActiveWindowTitle("REGEX3-R2D2");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);
QCOMPARE(m_test->actionChars(), QString("regex3"));
m_test->clearActions();

// with custom attributes
m_test->setActiveWindowTitle("CustomAttr1");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);
QCOMPARE(m_test->actionChars(), QString("custom_attr:Attribute"));
m_test->clearActions();

// with (non uppercase) undefined custom attributes
m_test->setActiveWindowTitle("CustomAttr2");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);
QCOMPARE(m_test->actionChars(), QString(""));
m_test->clearActions();

// with mixedcase default attributes
m_test->setActiveWindowTitle("CustomAttr3");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);
QCOMPARE(m_test->actionChars(), QString("custom_attr"));
m_test->clearActions();

// with resolve placeholders in window association title
m_test->setActiveWindowTitle("AttrValueFirst");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);
QCOMPARE(m_test->actionChars(), QString("custom_attr_first"));
m_test->clearActions();

m_test->setActiveWindowTitle("lorem AttrValueFirstAndAttrValueSecond ipsum");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);
QCOMPARE(m_test->actionChars(), QString("custom_attr_first_and_second"));
m_test->clearActions();

m_test->setActiveWindowTitle("lorem AttrValueThird ipsum");
m_test->triggerGlobalAutoType();
m_autoType->performGlobalAutoType(m_dbList);
QCOMPARE(m_test->actionChars(), QString("custom_attr_third"));
m_test->clearActions();
Expand Down

0 comments on commit cdc51f3

Please sign in to comment.