Correct issues with TOTP Setup

* Fix #3142 - Warn user when entering invalid TOTP secret key.
* Fix #773 - The TOTP dialog now listens for the copy shortcut without having to press the Copy button.

* Add ability to choose hash algorithm from the TOTP setup dialog
* Add upgrade to "otp" attribute when custom attributes are chosen to prevent data loss

src/core/Entry.cpp

@@ -442,16 +442,16 @@ QString Entry::totp() const
 void Entry::setTotp(QSharedPointer<Totp::Settings> settings)
 {
     beginUpdate();
+    m_attributes->remove(Totp::ATTRIBUTE_OTP);
+    m_attributes->remove(Totp::ATTRIBUTE_SEED);
+    m_attributes->remove(Totp::ATTRIBUTE_SETTINGS);
+
     if (settings->key.isEmpty()) {
         m_data.totpSettings.reset();
-        m_attributes->remove(Totp::ATTRIBUTE_OTP);
-        m_attributes->remove(Totp::ATTRIBUTE_SEED);
-        m_attributes->remove(Totp::ATTRIBUTE_SETTINGS);
     } else {
         m_data.totpSettings = std::move(settings);
-
         auto text = Totp::writeSettings(m_data.totpSettings, title(), username());
-        if (m_attributes->hasKey(Totp::ATTRIBUTE_OTP)) {
+        if (m_data.totpSettings->format != Totp::StorageFormat::LEGACY) {
             m_attributes->set(Totp::ATTRIBUTE_OTP, text, true);
         } else {
             m_attributes->set(Totp::ATTRIBUTE_SEED, m_data.totpSettings->key, true);
@@ -762,7 +762,8 @@ Entry* Entry::clone(CloneFlags flags) const
     entry->m_autoTypeAssociations->copyDataFrom(m_autoTypeAssociations);
     if (flags & CloneIncludeHistory) {
         for (Entry* historyItem : m_history) {
-            Entry* historyItemClone = historyItem->clone(flags & ~CloneIncludeHistory & ~CloneNewUuid & ~CloneResetTimeInfo);
+            Entry* historyItemClone =
+                historyItem->clone(flags & ~CloneIncludeHistory & ~CloneNewUuid & ~CloneResetTimeInfo);
             historyItemClone->setUpdateTimeinfo(false);
             historyItemClone->setUuid(entry->uuid());
             historyItemClone->setUpdateTimeinfo(true);

src/gui/TotpDialog.cpp

@@ -22,6 +22,9 @@
 #include "core/Clock.h"
 #include "core/Config.h"
 #include "gui/Clipboard.h"
+#include "gui/MainWindow.h"
+
+#include <QShortcut>
 
 TotpDialog::TotpDialog(QWidget* parent, Entry* entry)
     : QDialog(parent)
@@ -39,13 +42,16 @@ TotpDialog::TotpDialog(QWidget* parent, Entry* entry)
     resetCounter();
     updateProgressBar();
 
+    connect(parent, SIGNAL(lockedDatabase()), SLOT(close()));
     connect(&m_totpUpdateTimer, SIGNAL(timeout()), this, SLOT(updateProgressBar()));
     connect(&m_totpUpdateTimer, SIGNAL(timeout()), this, SLOT(updateSeconds()));
     m_totpUpdateTimer.start(m_step * 10);
     updateTotp();
 
     setAttribute(Qt::WA_DeleteOnClose);
 
+    new QShortcut(QKeySequence(QKeySequence::Copy), this, SLOT(copyToClipboard()));
+
     m_ui->buttonBox->button(QDialogButtonBox::Ok)->setText(tr("Copy"));
 
     connect(m_ui->buttonBox, SIGNAL(rejected()), SLOT(close()));
@@ -61,9 +67,9 @@ void TotpDialog::copyToClipboard()
     clipboard()->setText(m_entry->totp());
     if (config()->get("HideWindowOnCopy").toBool()) {
         if (config()->get("MinimizeOnCopy").toBool()) {
-            qobject_cast<DatabaseWidget*>(parent())->window()->showMinimized();
+            getMainWindow()->showMinimized();
         } else if (config()->get("DropToBackgroundOnCopy").toBool()) {
-            qobject_cast<DatabaseWidget*>(parent())->window()->lower();
+            getMainWindow()->lower();
             window()->lower();
         }
     }

src/gui/TotpExportSettingsDialog.cpp

@@ -16,10 +16,12 @@
  */
 
 #include "TotpExportSettingsDialog.h"
+
 #include "core/Config.h"
 #include "core/Entry.h"
 #include "gui/Clipboard.h"
 #include "gui/DatabaseWidget.h"
+#include "gui/MainWindow.h"
 #include "gui/SquareSvgWidget.h"
 #include "qrcode/QrCode.h"
 #include "totp/totp.h"
@@ -29,6 +31,7 @@
 #include <QLabel>
 #include <QMessageBox>
 #include <QPushButton>
+#include <QShortcut>
 #include <QSizePolicy>
 #include <QVBoxLayout>
 
@@ -55,10 +58,13 @@ TotpExportSettingsDialog::TotpExportSettingsDialog(DatabaseWidget* parent, Entry
 
     connect(m_buttonBox, SIGNAL(rejected()), SLOT(close()));
     connect(m_buttonBox, SIGNAL(accepted()), SLOT(copyToClipboard()));
-    connect(m_timer, SIGNAL(timeout()), this, SLOT(autoClose()));
-    connect(parent, SIGNAL(lockedDatabase()), this, SLOT(close()));
+    connect(m_timer, SIGNAL(timeout()), SLOT(autoClose()));
+    connect(parent, SIGNAL(lockedDatabase()), SLOT(close()));
+
+    new QShortcut(QKeySequence(QKeySequence::Copy), this, SLOT(copyToClipboard()));
 
     m_buttonBox->button(QDialogButtonBox::Ok)->setText(tr("Copy"));
+    m_buttonBox->setFocus();
     m_countDown->setAlignment(Qt::AlignCenter);
 
     m_secTillClose = 45;
@@ -92,18 +98,16 @@ TotpExportSettingsDialog::TotpExportSettingsDialog(DatabaseWidget* parent, Entry
         errorBox->exec();
         close();
     }
-
-    show();
 }
 
 void TotpExportSettingsDialog::copyToClipboard()
 {
     clipboard()->setText(m_totpUri);
     if (config()->get("HideWindowOnCopy").toBool()) {
         if (config()->get("MinimizeOnCopy").toBool()) {
-            static_cast<DatabaseWidget*>(parent())->window()->showMinimized();
+            getMainWindow()->showMinimized();
         } else if (config()->get("DropToBackgroundOnCopy").toBool()) {
-            static_cast<DatabaseWidget*>(parent())->window()->lower();
+            getMainWindow()->lower();
             window()->lower();
         }
     }

src/gui/TotpSetupDialog.cpp

@@ -19,6 +19,8 @@
 #include "TotpSetupDialog.h"
 #include "ui_TotpSetupDialog.h"
 
+#include "core/Base32.h"
+#include "gui/MessageBox.h"
 #include "totp/totp.h"
 
 TotpSetupDialog::TotpSetupDialog(QWidget* parent, Entry* entry)
@@ -43,51 +45,92 @@ TotpSetupDialog::~TotpSetupDialog()
 
 void TotpSetupDialog::saveSettings()
 {
+    // Secret key sanity check
+    auto key = m_ui->seedEdit->text().toLatin1();
+    auto sanitizedKey = Base32::sanitizeInput(key);
+    if (sanitizedKey != key) {
+        MessageBox::information(this,
+                                "Invalid TOTP Secret",
+                                "You have entered an invalid secret key. The key must be in Base32 format.\n"
+                                "Example: JBSWY3DPEHPK3PXP");
+        return;
+    }
+
     QString encShortName;
     uint digits = Totp::DEFAULT_DIGITS;
     uint step = Totp::DEFAULT_STEP;
+    Totp::Algorithm algorithm = Totp::DEFAULT_ALGORITHM;
+    Totp::StorageFormat format = Totp::DEFAULT_FORMAT;
 
     if (m_ui->radioSteam->isChecked()) {
         digits = Totp::STEAM_DIGITS;
         encShortName = Totp::STEAM_SHORTNAME;
     } else if (m_ui->radioCustom->isChecked()) {
+        algorithm = static_cast<Totp::Algorithm>(m_ui->algorithmComboBox->currentData().toInt());
         step = m_ui->stepSpinBox->value();
-        if (m_ui->radio8Digits->isChecked()) {
-            digits = 8;
-        } else if (m_ui->radio7Digits->isChecked()) {
-            digits = 7;
+        digits = m_ui->digitsSpinBox->value();
+    }
+
+    auto settings = m_entry->totpSettings();
+    if (settings) {
+        if (key.isEmpty()) {
+            auto answer = MessageBox::question(this,
+                                               "Confirm Remove TOTP Settings",
+                                               "Are you sure you want to delete TOTP settings for this entry?",
+                                               MessageBox::Delete | MessageBox::Cancel);
+            if (answer == MessageBox::Cancel) {
+                return;
+            }
+        }
+
+        format = settings->format;
+        if (format == Totp::StorageFormat::LEGACY && m_ui->radioCustom->isChecked()) {
+            auto answer = MessageBox::question(
+                this,
+                "Upgrade TOTP Settings",
+                "You are currently using legacy TOTP settings which do not fully support custom settings.\n"
+                "Would you like to upgrade to the latest format?",
+                MessageBox::Yes | MessageBox::No,
+                MessageBox::No);
+            if (answer == MessageBox::Yes) {
+                format = Totp::DEFAULT_FORMAT;
+            }
         }
     }
 
-    auto settings = Totp::createSettings(
-        m_ui->seedEdit->text(), digits, step, encShortName, Totp::HashType::Sha1, m_entry->totpSettings());
-    m_entry->setTotp(settings);
+    m_entry->setTotp(Totp::createSettings(key, digits, step, format, encShortName, algorithm));
     emit totpUpdated();
     close();
 }
 
 void TotpSetupDialog::toggleCustom(bool status)
 {
-    m_ui->customGroup->setEnabled(status);
+    m_ui->customSettingsGroup->setEnabled(status);
 }
 
 void TotpSetupDialog::init()
 {
+    // Add algorithm choices
+    auto algorithms = Totp::supportedAlgorithms();
+    for (const auto& item : algorithms) {
+        m_ui->algorithmComboBox->addItem(item.first, item.second);
+    }
+    m_ui->algorithmComboBox->setCurrentIndex(0);
+
+    // Read entry totp settings
     auto settings = m_entry->totpSettings();
-    if (!settings.isNull()) {
+    if (settings) {
         m_ui->seedEdit->setText(settings->key);
         m_ui->stepSpinBox->setValue(settings->step);
 
         if (settings->encoder.shortName == Totp::STEAM_SHORTNAME) {
             m_ui->radioSteam->setChecked(true);
         } else if (settings->custom) {
             m_ui->radioCustom->setChecked(true);
-            if (settings->digits == 8) {
-                m_ui->radio8Digits->setChecked(true);
-            } else if (settings->digits == 7) {
-                m_ui->radio7Digits->setChecked(true);
-            } else {
-                m_ui->radio6Digits->setChecked(true);
+            m_ui->digitsSpinBox->setValue(settings->digits);
+            int index = m_ui->algorithmComboBox->findData(settings->algorithm);
+            if (index != -1) {
+                m_ui->algorithmComboBox->setCurrentIndex(index);
             }
         }
     }