mirror of
https://github.com/keepassxreboot/keepassxc.git
synced 2024-10-01 01:26:01 -04:00
Fix multiple TOTP issues
* Fix #9847 - don't provide TOTP values if settings are blank or completely wrong * Fix #6838 - don't reset the ui when creating a new entry and applying TOTP to it * Move totp source into the core folder
This commit is contained in:
parent
5d64292ed8
commit
9f3b4dc5ea
@ -59,6 +59,7 @@ set(keepassx_SOURCES
|
||||
core/TimeDelta.cpp
|
||||
core/TimeInfo.cpp
|
||||
core/Tools.cpp
|
||||
core/Totp.cpp
|
||||
core/Translator.cpp
|
||||
core/UrlTools.cpp
|
||||
cli/Utils.cpp
|
||||
@ -194,8 +195,7 @@ set(keepassx_SOURCES
|
||||
streams/qtiocompressor.cpp
|
||||
streams/StoreDataStream.cpp
|
||||
streams/SymmetricCipherStream.cpp
|
||||
quickunlock/QuickUnlockInterface.cpp
|
||||
totp/totp.cpp)
|
||||
quickunlock/QuickUnlockInterface.cpp)
|
||||
if(APPLE)
|
||||
set(keepassx_SOURCES
|
||||
${keepassx_SOURCES}
|
||||
|
@ -24,7 +24,7 @@
|
||||
#include "core/Metadata.h"
|
||||
#include "core/PasswordHealth.h"
|
||||
#include "core/Tools.h"
|
||||
#include "totp/totp.h"
|
||||
#include "core/Totp.h"
|
||||
|
||||
#include <QDir>
|
||||
#include <QRegularExpression>
|
||||
@ -566,7 +566,7 @@ void Entry::setTotp(QSharedPointer<Totp::Settings> settings)
|
||||
m_attributes->remove(Totp::ATTRIBUTE_SEED);
|
||||
m_attributes->remove(Totp::ATTRIBUTE_SETTINGS);
|
||||
|
||||
if (settings->key.isEmpty()) {
|
||||
if (!settings || settings->key.isEmpty()) {
|
||||
m_data.totpSettings.reset();
|
||||
} else {
|
||||
m_data.totpSettings = std::move(settings);
|
||||
|
@ -16,7 +16,7 @@
|
||||
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
#include "totp.h"
|
||||
#include "Totp.h"
|
||||
|
||||
#include "core/Base32.h"
|
||||
#include "core/Clock.h"
|
||||
@ -59,6 +59,11 @@ static QString getNameForHashType(const Totp::Algorithm hashType)
|
||||
|
||||
QSharedPointer<Totp::Settings> Totp::parseSettings(const QString& rawSettings, const QString& key)
|
||||
{
|
||||
// Early out if both strings are empty
|
||||
if (rawSettings.isEmpty() && key.isEmpty()) {
|
||||
return {};
|
||||
}
|
||||
|
||||
// Create default settings
|
||||
auto settings = createSettings(key, DEFAULT_DIGITS, DEFAULT_STEP);
|
||||
|
||||
@ -96,6 +101,11 @@ QSharedPointer<Totp::Settings> Totp::parseSettings(const QString& rawSettings, c
|
||||
settings->algorithm = getHashTypeByName(query.queryItemValue("otpHashMode"));
|
||||
}
|
||||
} else {
|
||||
if (settings->key.isEmpty()) {
|
||||
// Legacy format cannot work with an empty key
|
||||
return {};
|
||||
}
|
||||
|
||||
// Parse semi-colon separated values ([step];[digits|S])
|
||||
settings->format = StorageFormat::LEGACY;
|
||||
auto vars = rawSettings.split(";");
|
@ -18,7 +18,7 @@
|
||||
#include "OpVaultReader.h"
|
||||
|
||||
#include "core/Entry.h"
|
||||
#include "totp/totp.h"
|
||||
#include "core/Totp.h"
|
||||
|
||||
#include <QDebug>
|
||||
#include <QJsonArray>
|
||||
|
@ -497,6 +497,10 @@ void DatabaseWidget::setupTotp()
|
||||
|
||||
auto setupTotpDialog = new TotpSetupDialog(this, currentEntry);
|
||||
connect(setupTotpDialog, SIGNAL(totpUpdated()), SIGNAL(entrySelectionChanged()));
|
||||
if (currentWidget() == m_editEntryWidget) {
|
||||
// Entry is being edited, tell it when we are finished updating TOTP
|
||||
connect(setupTotpDialog, SIGNAL(totpUpdated()), m_editEntryWidget, SLOT(updateTotp()));
|
||||
}
|
||||
connect(this, &DatabaseWidget::databaseLockRequested, setupTotpDialog, &TotpSetupDialog::close);
|
||||
setupTotpDialog->open();
|
||||
}
|
||||
|
@ -21,10 +21,10 @@
|
||||
|
||||
#include "Application.h"
|
||||
#include "core/Config.h"
|
||||
#include "core/Totp.h"
|
||||
#include "gui/Clipboard.h"
|
||||
#include "gui/Font.h"
|
||||
#include "gui/Icons.h"
|
||||
#include "totp/totp.h"
|
||||
#if defined(WITH_XC_KEESHARE)
|
||||
#include "keeshare/KeeShare.h"
|
||||
#include "keeshare/KeeShareSettings.h"
|
||||
|
@ -20,9 +20,9 @@
|
||||
#include "ui_TotpDialog.h"
|
||||
|
||||
#include "core/Clock.h"
|
||||
#include "core/Totp.h"
|
||||
#include "gui/Clipboard.h"
|
||||
#include "gui/MainWindow.h"
|
||||
#include "totp/totp.h"
|
||||
|
||||
#include <QPushButton>
|
||||
#include <QShortcut>
|
||||
|
@ -17,11 +17,11 @@
|
||||
|
||||
#include "TotpExportSettingsDialog.h"
|
||||
|
||||
#include "core/Totp.h"
|
||||
#include "gui/Clipboard.h"
|
||||
#include "gui/MainWindow.h"
|
||||
#include "gui/SquareSvgWidget.h"
|
||||
#include "qrcode/QrCode.h"
|
||||
#include "totp/totp.h"
|
||||
|
||||
#include <QBoxLayout>
|
||||
#include <QBuffer>
|
||||
|
@ -19,8 +19,8 @@
|
||||
#include "ui_TotpSetupDialog.h"
|
||||
|
||||
#include "core/Base32.h"
|
||||
#include "core/Totp.h"
|
||||
#include "gui/MessageBox.h"
|
||||
#include "totp/totp.h"
|
||||
|
||||
TotpSetupDialog::TotpSetupDialog(QWidget* parent, Entry* entry)
|
||||
: QDialog(parent)
|
||||
|
@ -22,9 +22,9 @@
|
||||
#include <QStringListModel>
|
||||
|
||||
#include "core/Clock.h"
|
||||
#include "core/Totp.h"
|
||||
#include "format/KeePass2Writer.h"
|
||||
#include "gui/MessageBox.h"
|
||||
#include "totp/totp.h"
|
||||
|
||||
// I wanted to make the CSV import GUI future-proof, so if one day you need a new field,
|
||||
// all you have to do is add a field to m_columnHeader, and the GUI will follow:
|
||||
@ -206,7 +206,7 @@ void CsvImportWidget::writeDatabase()
|
||||
auto otpString = m_parserModel->data(m_parserModel->index(r, 6));
|
||||
if (otpString.isValid() && !otpString.toString().isEmpty()) {
|
||||
auto totp = Totp::parseSettings(otpString.toString());
|
||||
if (totp->key.isEmpty()) {
|
||||
if (!totp || totp->key.isEmpty()) {
|
||||
// Bare secret, use default TOTP settings
|
||||
totp = Totp::parseSettings({}, otpString.toString());
|
||||
}
|
||||
|
@ -115,6 +115,7 @@ EditEntryWidget::EditEntryWidget(QWidget* parent)
|
||||
m_entryModifiedTimer.setSingleShot(true);
|
||||
m_entryModifiedTimer.setInterval(0);
|
||||
connect(&m_entryModifiedTimer, &QTimer::timeout, this, [this] {
|
||||
// TODO: Upon refactor of this widget, this needs to merge unsaved changes in the UI
|
||||
if (isVisible() && m_entry) {
|
||||
setForms(m_entry);
|
||||
}
|
||||
@ -711,6 +712,13 @@ void EditEntryWidget::toKeeAgentSettings(KeeAgentSettings& settings) const
|
||||
settings.setSaveAttachmentToTempFile(m_sshAgentSettings.saveAttachmentToTempFile());
|
||||
}
|
||||
|
||||
void EditEntryWidget::updateTotp()
|
||||
{
|
||||
if (m_entry) {
|
||||
m_attributesModel->setEntryAttributes(m_entry->attributes());
|
||||
}
|
||||
}
|
||||
|
||||
void EditEntryWidget::browsePrivateKey()
|
||||
{
|
||||
auto fileName = fileDialog()->getOpenFileName(this, tr("Select private key"), FileDialog::getLastDir("sshagent"));
|
||||
@ -867,8 +875,6 @@ void EditEntryWidget::loadEntry(Entry* entry,
|
||||
m_create = create;
|
||||
m_history = history;
|
||||
|
||||
connect(m_entry, &Entry::modified, this, [this] { m_entryModifiedTimer.start(); });
|
||||
|
||||
if (history) {
|
||||
setHeadline(QString("%1 \u2022 %2").arg(parentName, tr("Entry history")));
|
||||
} else {
|
||||
@ -876,6 +882,8 @@ void EditEntryWidget::loadEntry(Entry* entry,
|
||||
setHeadline(QString("%1 \u2022 %2").arg(parentName, tr("Add entry")));
|
||||
} else {
|
||||
setHeadline(QString("%1 \u2022 %2 \u2022 %3").arg(parentName, entry->title(), tr("Edit entry")));
|
||||
// Reload entry details if changed outside of the edit dialog
|
||||
connect(m_entry, &Entry::modified, this, [this] { m_entryModifiedTimer.start(); });
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -118,6 +118,7 @@ private slots:
|
||||
void updateSSHAgentAttachment();
|
||||
void updateSSHAgentAttachments();
|
||||
void updateSSHAgentKeyInfo();
|
||||
void updateTotp();
|
||||
void browsePrivateKey();
|
||||
void addKeyToAgent();
|
||||
void removeKeyFromAgent();
|
||||
|
@ -22,9 +22,9 @@
|
||||
#include <QTest>
|
||||
|
||||
#include "core/Group.h"
|
||||
#include "core/Totp.h"
|
||||
#include "crypto/Crypto.h"
|
||||
#include "format/CsvExporter.h"
|
||||
#include "totp/totp.h"
|
||||
|
||||
QTEST_GUILESS_MAIN(TestCsvExporter)
|
||||
|
||||
|
@ -20,9 +20,9 @@
|
||||
#include "config-keepassx-tests.h"
|
||||
#include "core/Group.h"
|
||||
#include "core/Metadata.h"
|
||||
#include "core/Totp.h"
|
||||
#include "crypto/Crypto.h"
|
||||
#include "format/OpVaultReader.h"
|
||||
#include "totp/totp.h"
|
||||
|
||||
#include <QJsonObject>
|
||||
#include <QList>
|
||||
|
@ -19,8 +19,8 @@
|
||||
#include "TestTotp.h"
|
||||
|
||||
#include "core/Entry.h"
|
||||
#include "core/Totp.h"
|
||||
#include "crypto/Crypto.h"
|
||||
#include "totp/totp.h"
|
||||
|
||||
#include <QTest>
|
||||
|
||||
@ -97,6 +97,14 @@ void TestTotp::testParseSecret()
|
||||
QCOMPARE(settings->digits, 6u);
|
||||
QCOMPARE(settings->step, 30u);
|
||||
QCOMPARE(settings->algorithm, Totp::Algorithm::Sha1);
|
||||
|
||||
// Blank settings (expected failure)
|
||||
settings = Totp::parseSettings("", "");
|
||||
QVERIFY(settings.isNull());
|
||||
|
||||
// TOTP Settings with blank secret (expected failure)
|
||||
settings = Totp::parseSettings("30;8", "");
|
||||
QVERIFY(settings.isNull());
|
||||
}
|
||||
|
||||
void TestTotp::testTotpCode()
|
||||
@ -164,4 +172,8 @@ void TestTotp::testEntryHistory()
|
||||
entry.setTotp(settings);
|
||||
QCOMPARE(entry.historyItems().size(), 2);
|
||||
QCOMPARE(entry.totpSettings()->key, QString("foo"));
|
||||
// Nullptr Settings (expected reset of TOTP)
|
||||
entry.setTotp(nullptr);
|
||||
QVERIFY(!entry.hasTotp());
|
||||
QCOMPARE(entry.historyItems().size(), 3);
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user