Skip to content

Commit

Permalink
Allow selecting any open database in unlock dialog
Browse files Browse the repository at this point in the history
When showing the database unlock dialog from browser or auto-type and
there's more than one open database, replace the filename label with
a drop-down box of all available files. Switching the filename here will
change the active tab of the main window and unlock it.

This implements the major feature desired by [1] and the noticeable UX
regression between 2.5 and 2.6 when working with multiple databases on
an auto-lock timer.

Notes:
  * Make the DatabaseOpenDialog window ApplicationModal so that it
    blocks input to the main UI when open. This avoids a number of
    corner cases that would otherwise pop up if opening/closing or
    locking/unlocking extra database tabs in the background.
  * This does not handle auto-type/browser scenarios where the active
    database is unlocked but has no matching credentials, and there's
    other locked databases open which the user could be prompted to
    unlock ("Case 2" in the issue).

[1] #2322
  • Loading branch information
aswild committed Sep 13, 2020
1 parent 767b193 commit f26f09b
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 3 deletions.
59 changes: 59 additions & 0 deletions src/gui/DatabaseOpenDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "DatabaseOpenDialog.h"
#include "DatabaseOpenWidget.h"
#include "DatabaseTabWidget.h"
#include "DatabaseWidget.h"
#include "core/Database.h"

Expand All @@ -26,6 +27,7 @@ DatabaseOpenDialog::DatabaseOpenDialog(QWidget* parent)
{
setWindowTitle(tr("Unlock Database - KeePassXC"));
setWindowFlags(Qt::Dialog | Qt::WindowStaysOnTopHint);
setWindowModality(Qt::ApplicationModal);
connect(m_view, SIGNAL(dialogFinished(bool)), this, SLOT(complete(bool)));
auto* layout = new QVBoxLayout();
layout->setMargin(0);
Expand All @@ -39,6 +41,42 @@ void DatabaseOpenDialog::setFilePath(const QString& filePath)
m_view->load(filePath);
}

/**
* This method toggles multi-file mode on the open dialog. In multi-file mode, the database path is
* presented as a drop-down list of all open locked databases in the parent DatabaseTabWidget (if
* there is more than one).
*
* Changing the selected database in the drop-down list will change the active tab in the main UI,
* so that browser and auto-type searches the current/active database appropriately.
*
* This method should be called *before* setFilePath. setMultiFile on its own doesn't reload the
* child DatabaseOpenWidget view.
*
* @param multiFile Whether to enable/disable multiFile mode (default is disabled)
*/
void DatabaseOpenDialog::setMultiFile(bool multiFile)
{
if (multiFile) {
auto* tabWidget = qobject_cast<const DatabaseTabWidget*>(parentWidget());
if (!tabWidget) {
qWarning("DatabaseOpenDialog::setMultiFile: parent widget is unset or invalid");
m_view->clearMultiFileList();
return;
}
QStringList fileList;
for (int i = 0, c = tabWidget->count(); i < c; ++i) {
auto* dbWidget = tabWidget->databaseWidgetFromIndex(i);
Q_ASSERT(dbWidget);
if (dbWidget && dbWidget->isLocked()) {
fileList << dbWidget->database()->filePath();
}
}
m_view->setMultiFileList(fileList);
} else {
m_view->clearMultiFileList();
}
}

/**
* Set target DatabaseWidget to which signals are connected.
*
Expand Down Expand Up @@ -92,3 +130,24 @@ void DatabaseOpenDialog::complete(bool accepted)
emit dialogFinished(accepted, m_dbWidget);
clearForms();
}

void DatabaseOpenDialog::changeFile(const QString& filePath)
{
auto* tabWidget = qobject_cast<DatabaseTabWidget*>(parentWidget());
if (!tabWidget) {
qWarning("DatabaseOpenDialog::changeFile: parent widget is unset or invalid");
return;
}

auto* dbWidget = tabWidget->databaseWidgetFromFilePath(filePath);
if (dbWidget) {
setTargetDatabaseWidget(dbWidget);
// make the newly-selected database active in the UI
tabWidget->setCurrentIndex(tabWidget->indexOf(dbWidget));
setFilePath(filePath);
} else {
// no matching dbWidget for the filePath, it probably got closed while this dialog was open.
// Cancel and let the user start over.
complete(false);
}
}
2 changes: 2 additions & 0 deletions src/gui/DatabaseOpenDialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class DatabaseOpenDialog : public QDialog

explicit DatabaseOpenDialog(QWidget* parent = nullptr);
void setFilePath(const QString& filePath);
void setMultiFile(bool multiFile);
void setTargetDatabaseWidget(DatabaseWidget* dbWidget);
void setIntent(Intent intent);
Intent intent() const;
Expand All @@ -54,6 +55,7 @@ class DatabaseOpenDialog : public QDialog

public slots:
void complete(bool accepted);
void changeFile(const QString& filePath);

private:
QPointer<DatabaseOpenWidget> m_view;
Expand Down
42 changes: 40 additions & 2 deletions src/gui/DatabaseOpenWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "core/Resources.h"
#include "crypto/Random.h"
#include "format/KeePass2Reader.h"
#include "gui/DatabaseOpenDialog.h"
#include "gui/FileDialog.h"
#include "gui/MainWindow.h"
#include "gui/MessageBox.h"
Expand Down Expand Up @@ -143,9 +144,30 @@ void DatabaseOpenWidget::load(const QString& filename)
clearForms();

m_filename = filename;
m_ui->fileNameLabel->setRawText(m_filename);

if (m_multiFileList.count() > 1) {
m_ui->fileNameLabel->setVisible(false);
m_ui->fileNameList->addItems(m_multiFileList);
m_ui->fileNameList->setCurrentText(filename);
m_ui->fileNameList->setVisible(true);

// changing the file in a dropdown needs to go back up the parent tree to change the current database tab.
// The handler of this signal will come back and call load() here again.
auto* parentDialog = qobject_cast<DatabaseOpenDialog*>(parentWidget());
if (parentDialog) {
// clang-format off
connect(m_ui->fileNameList, SIGNAL(textActivated(const QString&)),
parentDialog, SLOT(changeFile(const QString&)));
// clang-format on
}
} else {
m_ui->fileNameLabel->setRawText(m_filename);
m_ui->fileNameLabel->setVisible(true);
m_ui->fileNameList->setVisible(false);
}

m_ui->keyFileClearIcon->setVisible(false);
m_ui->editPassword->setFocus();

if (config()->get(Config::RememberLastKeyFiles).toBool()) {
auto lastKeyFiles = config()->get(Config::LastKeyFiles).toHash();
Expand All @@ -168,8 +190,24 @@ void DatabaseOpenWidget::load(const QString& filename)
#endif
}

/**
* Set a list of files in a drop-down to choose a different database.
* Must be called before load(), and should only be called by DatabaseOpenDialog
*/
void DatabaseOpenWidget::setMultiFileList(const QStringList& filenameList)
{
m_multiFileList = filenameList;
}

void DatabaseOpenWidget::clearMultiFileList()
{
m_multiFileList.clear();
}

void DatabaseOpenWidget::clearForms()
{
disconnect(m_ui->fileNameList, SIGNAL(textActivated(const QString&)), nullptr, nullptr);
m_ui->fileNameList->clear();
m_ui->editPassword->setText("");
m_ui->editPassword->setShowPassword(false);
m_ui->keyFileLineEdit->clear();
Expand All @@ -183,7 +221,7 @@ QSharedPointer<Database> DatabaseOpenWidget::database()
return m_db;
}

QString DatabaseOpenWidget::filename()
QString DatabaseOpenWidget::filename() const
{
return m_filename;
}
Expand Down
5 changes: 4 additions & 1 deletion src/gui/DatabaseOpenWidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ class DatabaseOpenWidget : public DialogyWidget
explicit DatabaseOpenWidget(QWidget* parent = nullptr);
~DatabaseOpenWidget();
void load(const QString& filename);
QString filename();
void setMultiFileList(const QStringList& filenameList);
void clearMultiFileList();
QString filename() const;
void clearForms();
void enterKey(const QString& pw, const QString& keyFile);
QSharedPointer<Database> database();
Expand All @@ -57,6 +59,7 @@ class DatabaseOpenWidget : public DialogyWidget
const QScopedPointer<Ui::DatabaseOpenWidget> m_ui;
QSharedPointer<Database> m_db;
QString m_filename;
QStringList m_multiFileList;
bool m_retryUnlockWithEmptyPassword = false;

protected slots:
Expand Down
7 changes: 7 additions & 0 deletions src/gui/DatabaseOpenWidget.ui
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@
</property>
</widget>
</item>
<item>
<widget class="QComboBox" name="fileNameList">
<property name="visible">
<bool>false</bool>
</property>
</widget>
</item>
<item>
<spacer name="verticalSpacer_3">
<property name="orientation">
Expand Down
14 changes: 14 additions & 0 deletions src/gui/DatabaseTabWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,18 @@ DatabaseWidget* DatabaseTabWidget::currentDatabaseWidget()
return qobject_cast<DatabaseWidget*>(currentWidget());
}

DatabaseWidget* DatabaseTabWidget::databaseWidgetFromFilePath(const QString& filePath) const
{
for (int i = 0, c = count(); i < c; ++i) {
auto* dbWidget = databaseWidgetFromIndex(i);
Q_ASSERT(dbWidget);
if (dbWidget && (dbWidget->database()->filePath() == filePath)) {
return dbWidget;
}
}
return nullptr;
}

/**
* Attempt to lock all open databases
*
Expand Down Expand Up @@ -666,6 +678,8 @@ void DatabaseTabWidget::unlockDatabaseInDialog(DatabaseWidget* dbWidget,
{
m_databaseOpenDialog->setTargetDatabaseWidget(dbWidget);
m_databaseOpenDialog->setIntent(intent);
m_databaseOpenDialog->setMultiFile(intent == DatabaseOpenDialog::Intent::AutoType
|| intent == DatabaseOpenDialog::Intent::Browser);
m_databaseOpenDialog->setFilePath(filePath);

#ifdef Q_OS_MACOS
Expand Down
1 change: 1 addition & 0 deletions src/gui/DatabaseTabWidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class DatabaseTabWidget : public QTabWidget
QString tabName(int index);
DatabaseWidget* currentDatabaseWidget();
DatabaseWidget* databaseWidgetFromIndex(int index) const;
DatabaseWidget* databaseWidgetFromFilePath(const QString& filePath) const;

bool isReadOnly(int index = -1) const;
bool canSave(int index = -1) const;
Expand Down

0 comments on commit f26f09b

Please sign in to comment.