Keychain: Use auto deletion in WebFlowCredentials and ConfigFile

- Also make use of the new KeychainChunk::DeleteJob

Signed-off-by: Michael Schuster <michael@schuster.ms>

diff --git a/src/gui/creds/webflowcredentials.cpp b/src/gui/creds/webflowcredentials.cpp
index 52eef03413e263faa9ee637814e40eb0747e973a..eaaebfc388bc44e0c2ed53bf2fe55b2cfa5a53fd 100644 (file)
--- a/src/gui/creds/webflowcredentials.cpp
+++ b/src/gui/creds/webflowcredentials.cpp
@@ -240,7 +240,8 @@ void WebFlowCredentials::persist() {
     if (!_clientSslCertificate.isNull()) {
         auto *job = new KeychainChunk::WriteJob(_account,
                                                 _user + clientCertificatePEMC,
-                                                _clientSslCertificate.toPem());
+                                                _clientSslCertificate.toPem(),
+                                                this);
         connect(job, &KeychainChunk::WriteJob::finished, this, &WebFlowCredentials::slotWriteClientCertPEMJobDone);
         job->start();
     } else {
@@ -251,14 +252,12 @@ void WebFlowCredentials::persist() {
 
 void WebFlowCredentials::slotWriteClientCertPEMJobDone(KeychainChunk::WriteJob *writeJob)
 {
-    if(writeJob)
-        writeJob->deleteLater();
-
     // write ssl key if there is one
     if (!_clientSslKey.isNull()) {
         auto *job = new KeychainChunk::WriteJob(_account,
                                                 _user + clientKeyPEMC,
-                                                _clientSslKey.toPem());
+                                                _clientSslKey.toPem(),
+                                                this);
         connect(job, &KeychainChunk::WriteJob::finished, this, &WebFlowCredentials::slotWriteClientKeyPEMJobDone);
         job->start();
     } else {
@@ -288,7 +287,8 @@ void WebFlowCredentials::writeSingleClientCaCertPEM()
 
         auto *job = new KeychainChunk::WriteJob(_account,
                                                 _user + clientCaCertificatePEMC + QString::number(index),
-                                                cert.toPem());
+                                                cert.toPem(),
+                                                this);
         connect(job, &KeychainChunk::WriteJob::finished, this, &WebFlowCredentials::slotWriteClientCaCertsPEMJobDone);
         job->start();
     } else {
@@ -298,9 +298,6 @@ void WebFlowCredentials::writeSingleClientCaCertPEM()
 
 void WebFlowCredentials::slotWriteClientKeyPEMJobDone(KeychainChunk::WriteJob *writeJob)
 {
-    if(writeJob)
-        writeJob->deleteLater();
-
     _clientSslCaCertificatesWriteQueue.clear();
 
     // write ca certs if there are any
@@ -323,8 +320,6 @@ void WebFlowCredentials::slotWriteClientCaCertsPEMJobDone(KeychainChunk::WriteJo
             qCWarning(lcWebFlowCredentials) << "Error while writing client CA cert" << writeJob->errorString();
         }
 
-        writeJob->deleteLater();
-
         if (!_clientSslCaCertificatesWriteQueue.isEmpty()) {
             // next ca cert
             writeSingleClientCaCertPEM();
@@ -333,7 +328,7 @@ void WebFlowCredentials::slotWriteClientCaCertsPEMJobDone(KeychainChunk::WriteJo
     }
 
     // done storing ca certs, time for the password
-    auto *job = new WritePasswordJob(Theme::instance()->appName());
+    auto *job = new WritePasswordJob(Theme::instance()->appName(), this);
 #if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
     addSettingsToJob(_account, job);
 #endif
@@ -353,8 +348,6 @@ void WebFlowCredentials::slotWriteJobDone(QKeychain::Job *job)
     default:
         qCWarning(lcWebFlowCredentials) << "Error while writing password" << job->errorString();
     }
-    auto *wjob = qobject_cast<WritePasswordJob *>(job);
-    wjob->deleteLater();
 }
 
 void WebFlowCredentials::invalidateToken() {
@@ -383,13 +376,9 @@ void WebFlowCredentials::forgetSensitiveData() {
         return;
     }
 
-    auto *job = new DeletePasswordJob(Theme::instance()->appName());
+    auto *job = new DeletePasswordJob(Theme::instance()->appName(), this);
     job->setInsecureFallback(false);
     job->setKey(kck);
-    connect(job, &Job::finished, this, [](QKeychain::Job *job) {
-        auto *djob = qobject_cast<DeletePasswordJob *>(job);
-        djob->deleteLater();
-    });
     job->start();
 
     invalidateToken();
@@ -442,7 +431,8 @@ void WebFlowCredentials::fetchFromKeychainHelper() {
     // Read client cert from keychain
     auto *job = new KeychainChunk::ReadJob(_account,
                                            _user + clientCertificatePEMC,
-                                           _keychainMigration);
+                                           _keychainMigration,
+                                           this);
     connect(job, &KeychainChunk::ReadJob::finished, this, &WebFlowCredentials::slotReadClientCertPEMJobDone);
     job->start();
 }
@@ -457,12 +447,11 @@ void WebFlowCredentials::slotReadClientCertPEMJobDone(KeychainChunk::ReadJob *re
         }
     }
 
-    readJob->deleteLater();
-
     // Load key too
     auto *job = new KeychainChunk::ReadJob(_account,
                                            _user + clientKeyPEMC,
-                                           _keychainMigration);
+                                           _keychainMigration,
+                                           this);
     connect(job, &KeychainChunk::ReadJob::finished, this, &WebFlowCredentials::slotReadClientKeyPEMJobDone);
     job->start();
 }
@@ -489,8 +478,6 @@ void WebFlowCredentials::slotReadClientKeyPEMJobDone(KeychainChunk::ReadJob *rea
         qCWarning(lcWebFlowCredentials) << "Unable to read client key" << readJob->errorString();
     }
 
-    readJob->deleteLater();
-
     // Start fetching client CA certs
     _clientSslCaCertificates.clear();
 
@@ -503,7 +490,8 @@ void WebFlowCredentials::readSingleClientCaCertPEM()
     if (_clientSslCaCertificates.count() < _clientSslCaCertificatesMaxCount) {
         auto *job = new KeychainChunk::ReadJob(_account,
                                                _user + clientCaCertificatePEMC + QString::number(_clientSslCaCertificates.count()),
-                                               _keychainMigration);
+                                               _keychainMigration,
+                                               this);
         connect(job, &KeychainChunk::ReadJob::finished, this, &WebFlowCredentials::slotReadClientCaCertsPEMJobDone);
         job->start();
     } else {
@@ -522,8 +510,6 @@ void WebFlowCredentials::slotReadClientCaCertsPEMJobDone(KeychainChunk::ReadJob
                 _clientSslCaCertificates.append(sslCertificateList.at(0));
             }
 
-            readJob->deleteLater();
-
             // try next cert
             readSingleClientCaCertPEM();
             return;
@@ -533,8 +519,6 @@ void WebFlowCredentials::slotReadClientCaCertsPEMJobDone(KeychainChunk::ReadJob
                 qCWarning(lcWebFlowCredentials) << "Unable to read client CA cert slot" << QString::number(_clientSslCaCertificates.count()) << readJob->errorString();
             }
         }
-
-        readJob->deleteLater();
     }
 
     // Now fetch the actual server password
@@ -543,7 +527,7 @@ void WebFlowCredentials::slotReadClientCaCertsPEMJobDone(KeychainChunk::ReadJob
         _user,
         _keychainMigration ? QString() : _account->id());
 
-    auto *job = new ReadPasswordJob(Theme::instance()->appName());
+    auto *job = new ReadPasswordJob(Theme::instance()->appName(), this);
 #if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
     addSettingsToJob(_account, job);
 #endif
@@ -577,8 +561,6 @@ void WebFlowCredentials::slotReadPasswordJobDone(Job *incomingJob) {
     }
     emit fetched();
 
-    job->deleteLater();
-
     // If keychain data was read from legacy location, wipe these entries and store new ones
     if (_keychainMigration && _ready) {
         _keychainMigration = false;
@@ -590,15 +572,7 @@ void WebFlowCredentials::slotReadPasswordJobDone(Job *incomingJob) {
 
 void WebFlowCredentials::deleteKeychainEntries(bool oldKeychainEntries) {
     auto startDeleteJob = [this, oldKeychainEntries](QString key) {
-        auto *job = new DeletePasswordJob(Theme::instance()->appName());
-#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
-        addSettingsToJob(_account, job);
-#endif
-        job->setInsecureFallback(false);
-        job->setKey(keychainKey(_account->url().toString(),
-                                key,
-                                oldKeychainEntries ? QString() : _account->id()));
-        job->setAutoDelete(true);
+        auto *job = new KeychainChunk::DeleteJob(_account, key, oldKeychainEntries, this);
         job->start();
     };
 
@@ -615,27 +589,17 @@ void WebFlowCredentials::deleteKeychainEntries(bool oldKeychainEntries) {
      */
     if(_account->isRemoteWipeRequested_HACK()) {
     // <-- FIXME MS@2019-12-07
+
+        // Also delete key / cert sub-chunks (KeychainChunk takes care of the Windows workaround)
+        // The first chunk (0) has no suffix, to stay compatible with older versions and non-Windows
         startDeleteJob(_user + clientKeyPEMC);
         startDeleteJob(_user + clientCertificatePEMC);
 
+        // CA cert slots
         for (auto i = 0; i < _clientSslCaCertificates.count(); i++) {
             startDeleteJob(_user + clientCaCertificatePEMC + QString::number(i));
         }
 
-#if defined(Q_OS_WIN)
-        // Also delete key / cert sub-chunks (Windows workaround)
-        // The first chunk (0) has no suffix, to stay compatible with older versions and non-Windows
-        for (auto chunk = 1; chunk < KeychainChunk::MaxChunks; chunk++) {
-            const QString strChunkSuffix = QString(".") + QString::number(chunk);
-
-            startDeleteJob(_user + clientKeyPEMC + strChunkSuffix);
-            startDeleteJob(_user + clientCertificatePEMC + strChunkSuffix);
-
-            for (auto i = 0; i < _clientSslCaCertificates.count(); i++) {
-                startDeleteJob(_user + clientCaCertificatePEMC + QString::number(i));
-            }
-        }
-#endif
     // FIXME MS@2019-12-07 -->
     }
     // <-- FIXME MS@2019-12-07

diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp
index 299ab09d1a6fe58534197ba2d8465be894562d8e..192f7cfbb9c0b426b44b2bca262f6577c277b205 100644 (file)
--- a/src/libsync/configfile.cpp
+++ b/src/libsync/configfile.cpp
@@ -654,20 +654,19 @@ void ConfigFile::setProxyType(int proxyType,
         settings.setValue(QLatin1String(proxyUserC), user);
 
         if (pass.isEmpty()) {
+            // Security: Don't keep password in config file
             settings.remove(QLatin1String(proxyPassC));
 
-            auto *job = new QKeychain::DeletePasswordJob(Theme::instance()->appName());
-            job->setInsecureFallback(false);
-            job->setKey(keychainProxyPasswordKey());
-            job->setAutoDelete(true);
-            job->start();
+            // Delete password from keychain
+            auto *job = new KeychainChunk::DeleteJob(keychainProxyPasswordKey());
+            job->exec();
         } else {
             // Write password to keychain
             auto *job = new KeychainChunk::WriteJob(keychainProxyPasswordKey(), pass.toUtf8());
-            if (job->startAwait()) {
+            if (job->exec()) {
+                // Security: Don't keep password in config file
                 settings.remove(QLatin1String(proxyPassC));
             }
-            job->deleteLater();
         }
     }
     settings.sync();
@@ -752,19 +751,17 @@ QString ConfigFile::proxyPassword() const
     if (!pass.isEmpty()) {
         // Security: Migrate password from config file to keychain
         auto *job = new KeychainChunk::WriteJob(key, pass.toUtf8());
-        if (job->startAwait()) {
+        if (job->exec()) {
             QSettings settings(configFile(), QSettings::IniFormat);
             settings.remove(QLatin1String(proxyPassC));
-            qCInfo(lcConfigFile()) << "Migrated proxy password to keychain for" << key;
+            qCInfo(lcConfigFile()) << "Migrated proxy password to keychain";
         }
-        job->deleteLater();
     } else {
         // Read password from keychain
         auto *job = new KeychainChunk::ReadJob(key);
-        if (job->startAwait()) {
+        if (job->exec()) {
             pass = job->textData();
         }
-        job->deleteLater();
     }
 
     return pass;