Fix Remote Wipe keychain storage
authorMichael Schuster <michael@schuster.ms>
Sat, 7 Dec 2019 23:02:11 +0000 (00:02 +0100)
committerMichael Schuster <48932272+misch7@users.noreply.github.com>
Sun, 8 Dec 2019 01:47:22 +0000 (02:47 +0100)
In certain cases don't write the app password in Account::writeAppPasswordOnce:
- id() is empty: This always happend once the Account Wizard showed the folder selection
- appPassword is empty: Caused by Logout -> Relaunch, preventing remote wipe on relaunch

Implement some logging to ease debugging in the future.

Signed-off-by: Michael Schuster <michael@schuster.ms>
src/libsync/account.cpp

index 75b722962e826930077f469df37f962a87624adb..076fb196d382155f14f53ab341237a7b9726ec26 100644 (file)
@@ -517,6 +517,14 @@ void Account::writeAppPasswordOnce(QString appPassword){
     if(_wroteAppPassword)
         return;
 
+    // Fix: Password got written from Account Wizard, before finish.
+    // Only write the app password for a connected account, else
+    // there'll be a zombie keychain slot forever, never used again ;p
+    //
+    // Also don't write empty passwords (Log out -> Relaunch)
+    if(id().isEmpty() || appPassword.isEmpty())
+        return;
+
     const QString kck = AbstractCredentials::keychainKey(
                 url().toString(),
                 davUser() + app_password,
@@ -527,9 +535,14 @@ void Account::writeAppPasswordOnce(QString appPassword){
     job->setInsecureFallback(false);
     job->setKey(kck);
     job->setBinaryData(appPassword.toLatin1());
-    connect(job, &WritePasswordJob::finished, [this](Job *) {
-        qCInfo(lcAccount) << "appPassword stored in keychain";
-
+    connect(job, &WritePasswordJob::finished, [this](Job *incoming) {
+        WritePasswordJob *writeJob = static_cast<WritePasswordJob *>(incoming);
+        if (writeJob->error() == NoError)
+            qCInfo(lcAccount) << "appPassword stored in keychain";
+        else
+            qCWarning(lcAccount) << "Unable to store appPassword in keychain" << writeJob->errorString();
+
+        // We don't try this again on error, to not raise CPU consumption
         _wroteAppPassword = true;
     });
     job->start();
@@ -574,6 +587,16 @@ void Account::deleteAppPassword(){
     DeletePasswordJob *job = new DeletePasswordJob(Theme::instance()->appName());
     job->setInsecureFallback(false);
     job->setKey(kck);
+    connect(job, &DeletePasswordJob::finished, [this](Job *incoming) {
+        DeletePasswordJob *deleteJob = static_cast<DeletePasswordJob *>(incoming);
+        if (deleteJob->error() == NoError)
+            qCInfo(lcAccount) << "appPassword deleted from keychain";
+        else
+            qCWarning(lcAccount) << "Unable to delete appPassword from keychain" << deleteJob->errorString();
+
+        // Allow storing a new app password on re-login
+        _wroteAppPassword = false;
+    });
     job->start();
 }