Improve: do not try load-/sav-ing password securely when storing them…

… portably

I was annoyed by all the error messages - both on command line from Mudlet
itself and also from (on Linux/FreeBSD) the "wallet" system about not finding
passwords from the secure storage system when I have explicitly said that
Mudlet is to use the (insecure) PORTABLE method. It is worth noting that the
second type of warnings (from the DE/OS) can be highly noticably as they are
popups that require clicking to dismiss them!

I also took the liberty of tweaking related command line debugging texts to be
formatted slightly differently so that they matched the format I now tend to
use: "class::method name(...) WORD - text." with terminal punctation;
insertion of double quotes around strings; and spaces as required manually
rather than letting the Qt QDebug class doing it on every element of a
message.

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

src/Host.cpp

@@ -459,7 +459,16 @@ Host::Host(int port, const QString& hostname, const QString& login, const QStrin
         mDiscordDisableServerSide = optin.toInt() == Qt::Unchecked ? true : false;
     }
 
-    loadSecuredPassword();
+    if (mudlet::self()->storingPasswordsSecurely()) {
+        // This will not insert the password into the mPass member immediately:
+        loadSecuredPassword();
+    } else {
+        QString password{readProfileData(QStringLiteral("password"))};
+        if (!password.isEmpty()) {
+            setPass(password);
+            password.clear();
+        }
+    }
 
     if (mudlet::scmIsPublicTestVersion) {
         thankForUsingPTB();
@@ -2692,6 +2701,7 @@ void Host::setDisplayFontFixedPitch(bool enable)
     mDisplayFont.setFixedPitch(enable);
 }
 
+// Only used in the constructor - and only when passwords are stored securely
 void Host::loadSecuredPassword()
 {
     auto *job = new QKeychain::ReadPasswordJob(QStringLiteral("Mudlet profile"));
@@ -2703,9 +2713,11 @@ void Host::loadSecuredPassword()
     connect(job, &QKeychain::ReadPasswordJob::finished, this, [=](QKeychain::Job* job) {
         if (job->error()) {
             const auto error = job->errorString();
+            // Report to debug output the problem if it was something other than the password not being found
             if (error != QStringLiteral("Entry not found") && error != QStringLiteral("No match")) {
-                qDebug() << "Host::loadSecuredPassword ERROR: couldn't retrieve secure password for" << getName() << ", error is:" << error;
+                qDebug().nospace().noquote() << "Host::loadSecuredPassword() ERROR - could not retrieve secure password for \"" << getName() << "\", error is: " << error << ".";
             }
+
         } else {
             auto readJob = static_cast<QKeychain::ReadPasswordJob*>(job);
             setPass(readJob->textData());

src/dlgConnectionProfiles.cpp

@@ -338,6 +338,7 @@ void dlgConnectionProfiles::writeSecurePassword(const QString& profile, const QS
     job->start();
 }
 
+// Only used by migrateSecuredPassword(...)
 void dlgConnectionProfiles::deleteSecurePassword(const QString& profile) const
 {
     auto* job = new QKeychain::DeletePasswordJob(QStringLiteral("Mudlet profile"));
@@ -490,7 +491,9 @@ void dlgConnectionProfiles::slot_save_name()
         return;
     }
 
-    migrateSecuredPassword(currentProfileEditName, newProfileName);
+    if (mudlet::self()->storingPasswordsSecurely()) {
+        migrateSecuredPassword(currentProfileEditName, newProfileName);
+    }
 
     setItemName(pItem, newProfileName);
 
@@ -908,13 +911,18 @@ void dlgConnectionProfiles::slot_item_clicked(QListWidgetItem* pItem)
     // by the copy method
     if (!mCopyingProfile) {
         character_password_entry->setText(QString());
-        loadSecuredPassword(profile_name, [this, profile_name](const QString& password) {
-            if (!password.isEmpty()) {
-                character_password_entry->setText(password);
-            } else {
-                character_password_entry->setText(readProfileData(profile_name, QStringLiteral("password")));
-            }
-        });
+        if (mudlet::self()->storingPasswordsSecurely()) {
+            loadSecuredPassword(profile_name, [this, profile_name](const QString& password) {
+                if (!password.isEmpty()) {
+                    character_password_entry->setText(password);
+                } else {
+                    character_password_entry->setText(readProfileData(profile_name, QStringLiteral("password")));
+                }
+            });
+
+        } else {
+            character_password_entry->setText(readProfileData(profile_name, QStringLiteral("password")));
+        }
     }
 
     val = readProfileData(profile_name, QStringLiteral("login"));
@@ -1229,7 +1237,9 @@ void dlgConnectionProfiles::setCustomIcon(const QString& profileName, QListWidge
     profile->setIcon(icon);
 }
 
-// When a profile is renamed, migrate password storage to the new profile
+// When a profile is renamed, migrate password storage to the new profile.
+// This is only used by slot_save_name(...) and that now only when passwords
+// are stored securely:
 void dlgConnectionProfiles::migrateSecuredPassword(const QString& oldProfile, const QString& newProfile)
 {
     const auto& password = character_password_entry->text().trimmed();
@@ -1253,8 +1263,10 @@ void dlgConnectionProfiles::loadSecuredPassword(const QString& profile, L callba
         if (job->error()) {
             const auto error = job->errorString();
             if (error != QStringLiteral("Entry not found") && error != QStringLiteral("No match")) {
-                qDebug() << "dlgConnectionProfiles::loadSecuredPassword ERROR: couldn't retrieve secure password for" << profile << ", error is:" << error;
+                // Report to debug output the problem if it was something other than a couple of 'normal' failure mechanisms:
+                qDebug().nospace().noquote() << "dlgConnectionProfiles::loadSecuredPassword() ERROR - could not retrieve secure password for \"" << profile << "\", error is: " << error << ".";
             }
+
         }
 
         auto readJob = static_cast<QKeychain::ReadPasswordJob*>(job);

src/mudlet.cpp

@@ -3804,7 +3804,7 @@ void mudlet::slot_password_migrated_to_secure(QKeychain::Job* job)
 bool mudlet::migratePasswordsToProfileStorage()
 {
     if (!mProfilePasswordsToMigrate.isEmpty()) {
-        qWarning() << "mudlet::migratePasswordsToProfileStorage() warning: password migration is already in progress, won't start another.";
+        qWarning() << "mudlet::migratePasswordsToProfileStorage() WARMING - password migration is already in progress, so not starting a duplicate action.";
         return false;
     }
     mStorePasswordsSecurely = false;
@@ -3838,8 +3838,9 @@ void mudlet::slot_password_migrated_to_profile(QKeychain::Job* job)
     if (job->error()) {
         const auto error = job->errorString();
         if (error != QStringLiteral("Entry not found") && error != QStringLiteral("No match")) {
-            qWarning() << "mudlet::slot_password_migrated_to_profile ERROR: couldn't migrate for" << profileName << "; error was:" << error;
+            qWarning().nospace().noquote() << "mudlet::slot_password_migrated_to_profile(...) ERROR - could not migrate for \"" << profileName << "\"; error was: " << error << ".";
         }
+
     } else {
         auto readJob = static_cast<QKeychain::ReadPasswordJob*>(job);
         writeProfileData(profileName, QStringLiteral("password"), readJob->textData());