Credentials: Use per-account keychain entries #5830

This requires a lot of migration code: the old entries need to be read,
saved to the new locations and then deleted.

src/gui/creds/shibbolethcredentials.cpp

@@ -54,6 +54,7 @@ ShibbolethCredentials::ShibbolethCredentials()
     , _ready(false)
     , _stillValid(false)
     , _browser(0)
+    , _keychainMigration(false)
 {
 }
 
@@ -62,6 +63,7 @@ ShibbolethCredentials::ShibbolethCredentials(const QNetworkCookie &cookie)
     , _stillValid(true)
     , _browser(0)
     , _shibCookie(cookie)
+    , _keychainMigration(false)
 {
 }
 
@@ -131,13 +133,20 @@ void ShibbolethCredentials::fetchFromKeychain()
         Q_EMIT fetched();
     } else {
         _url = _account->url();
+        _keychainMigration = false;
+        fetchFromKeychainHelper();
+    }
+}
+
+void ShibbolethCredentials::fetchFromKeychainHelper()
+{
     ReadPasswordJob *job = new ReadPasswordJob(Theme::instance()->appName());
     job->setSettings(ConfigFile::settingsWithGroup(Theme::instance()->appName(), job).release());
     job->setInsecureFallback(false);
-        job->setKey(keychainKey(_account->url().toString(), user()));
+    job->setKey(keychainKey(_url.toString(), user(),
+        _keychainMigration ? QString() : _account->id()));
     connect(job, SIGNAL(finished(QKeychain::Job *)), SLOT(slotReadJobDone(QKeychain::Job *)));
     job->start();
-    }
 }
 
 void ShibbolethCredentials::askFromUser()
@@ -242,6 +251,16 @@ void ShibbolethCredentials::slotBrowserRejected()
 
 void ShibbolethCredentials::slotReadJobDone(QKeychain::Job *job)
 {
+    // If we can't find the credentials at the keys that include the account id,
+    // try to read them from the legacy locations that don't have a account id.
+    if (!_keychainMigration && job->error() == QKeychain::EntryNotFound) {
+        qCWarning(lcShibboleth)
+            << "Could not find keychain entry, attempting to read from legacy location";
+        _keychainMigration = true;
+        fetchFromKeychainHelper();
+        return;
+    }
+
     if (job->error() == QKeychain::NoError) {
         ReadPasswordJob *readJob = static_cast<ReadPasswordJob *>(job);
         delete readJob->settings();
@@ -260,6 +279,19 @@ void ShibbolethCredentials::slotReadJobDone(QKeychain::Job *job)
         _ready = false;
         Q_EMIT fetched();
     }
+
+
+    // If keychain data was read from legacy location, wipe these entries and store new ones
+    if (_keychainMigration && _ready) {
+        persist();
+
+        DeletePasswordJob *job = new DeletePasswordJob(Theme::instance()->appName());
+        job->setSettings(ConfigFile::settingsWithGroup(Theme::instance()->appName(), job).release());
+        job->setKey(keychainKey(_account->url().toString(), user(), QString()));
+        job->start();
+
+        qCWarning(lcShibboleth) << "Migrated old keychain entries";
+    }
 }
 
 void ShibbolethCredentials::showLoginWindow()
@@ -313,7 +345,7 @@ void ShibbolethCredentials::storeShibCookie(const QNetworkCookie &cookie)
     job->setSettings(ConfigFile::settingsWithGroup(Theme::instance()->appName(), job).release());
     // we don't really care if it works...
     //connect(job, SIGNAL(finished(QKeychain::Job*)), SLOT(slotWriteJobDone(QKeychain::Job*)));
-    job->setKey(keychainKey(_account->url().toString(), user()));
+    job->setKey(keychainKey(_account->url().toString(), user(), _account->id()));
     job->setTextData(QString::fromUtf8(cookie.toRawForm()));
     job->start();
 }
@@ -322,7 +354,7 @@ void ShibbolethCredentials::removeShibCookie()
 {
     DeletePasswordJob *job = new DeletePasswordJob(Theme::instance()->appName());
     job->setSettings(ConfigFile::settingsWithGroup(Theme::instance()->appName(), job).release());
-    job->setKey(keychainKey(_account->url().toString(), user()));
+    job->setKey(keychainKey(_account->url().toString(), user(), _account->id()));
     job->start();
 }
 
@@ -336,5 +368,4 @@ void ShibbolethCredentials::addToCookieJar(const QNetworkCookie &cookie)
     jar->blockSignals(false);
 }
 
-
 } // namespace OCC

src/gui/creds/shibbolethcredentials.h

@@ -84,6 +84,10 @@ private:
     void storeShibCookie(const QNetworkCookie &cookie);
     void removeShibCookie();
     void addToCookieJar(const QNetworkCookie &cookie);
+
+    /// Reads data from keychain, progressing to slotReadJobDone
+    void fetchFromKeychainHelper();
+
     QUrl _url;
     QByteArray prepareCookieData() const;
 
@@ -92,6 +96,7 @@ private:
     QPointer<ShibbolethWebView> _browser;
     QNetworkCookie _shibCookie;
     QString _user;
+    bool _keychainMigration;
 };
 
 } // namespace OCC

src/libsync/creds/abstractcredentials.cpp

@@ -34,7 +34,7 @@ void AbstractCredentials::setAccount(Account *account)
     _account = account;
 }
 
-QString AbstractCredentials::keychainKey(const QString &url, const QString &user)
+QString AbstractCredentials::keychainKey(const QString &url, const QString &user, const QString &accountId)
 {
     QString u(url);
     if (u.isEmpty()) {
@@ -51,6 +51,9 @@ QString AbstractCredentials::keychainKey(const QString &url, const QString &user
     }
 
     QString key = user + QLatin1Char(':') + u;
+    if (!accountId.isEmpty()) {
+        key += QLatin1Char(':') + accountId;
+    }
     return key;
 }
 } // namespace OCC

src/libsync/creds/abstractcredentials.h

@@ -85,7 +85,7 @@ public:
      */
     virtual void forgetSensitiveData() = 0;
 
-    static QString keychainKey(const QString &url, const QString &user);
+    static QString keychainKey(const QString &url, const QString &user, const QString &accountId);
 
 Q_SIGNALS:
     /** Emitted when fetchFromKeychain() is done.

src/libsync/creds/httpcredentials.cpp

@@ -104,6 +104,7 @@ static void addSettingsToJob(Account *account, QKeychain::Job *job)
 
 HttpCredentials::HttpCredentials()
     : _ready(false)
+    , _keychainMigration(false)
 {
 }
 
@@ -114,6 +115,7 @@ HttpCredentials::HttpCredentials(const QString &user, const QString &password, c
     , _ready(true)
     , _clientSslKey(key)
     , _clientSslCertificate(certificate)
+    , _keychainMigration(false)
 {
 }
 
@@ -175,21 +177,43 @@ void HttpCredentials::fetchFromKeychain()
         return;
     }
 
-    const QString kck = keychainKey(_account->url().toString(), _user);
-
     if (_ready) {
         Q_EMIT fetched();
     } else {
+        _keychainMigration = false;
+        fetchFromKeychainHelper();
+    }
+}
+
+void HttpCredentials::fetchFromKeychainHelper()
+{
     // Read client cert from keychain
-        const QString kck = keychainKey(_account->url().toString(), _user + clientCertificatePEMC);
+    const QString kck = keychainKey(
+        _account->url().toString(),
+        _user + clientCertificatePEMC,
+        _keychainMigration ? QString() : _account->id());
+
     ReadPasswordJob *job = new ReadPasswordJob(Theme::instance()->appName());
     addSettingsToJob(_account, job);
     job->setInsecureFallback(false);
     job->setKey(kck);
-
     connect(job, SIGNAL(finished(QKeychain::Job *)), SLOT(slotReadClientCertPEMJobDone(QKeychain::Job *)));
     job->start();
-    }
+}
+
+void HttpCredentials::deleteOldKeychainEntries()
+{
+    auto startDeleteJob = [this](QString user) {
+        DeletePasswordJob *job = new DeletePasswordJob(Theme::instance()->appName());
+        addSettingsToJob(_account, job);
+        job->setInsecureFallback(true);
+        job->setKey(keychainKey(_account->url().toString(), user, QString()));
+        job->start();
+    };
+
+    startDeleteJob(_user);
+    startDeleteJob(_user + clientKeyPEMC);
+    startDeleteJob(_user + clientCertificatePEMC);
 }
 
 void HttpCredentials::slotReadClientCertPEMJobDone(QKeychain::Job *incoming)
@@ -204,12 +228,15 @@ void HttpCredentials::slotReadClientCertPEMJobDone(QKeychain::Job *incoming)
     }
 
     // Load key too
-    const QString kck = keychainKey(_account->url().toString(), _user + clientKeyPEMC);
+    const QString kck = keychainKey(
+        _account->url().toString(),
+        _user + clientKeyPEMC,
+        _keychainMigration ? QString() : _account->id());
+
     ReadPasswordJob *job = new ReadPasswordJob(Theme::instance()->appName());
     addSettingsToJob(_account, job);
     job->setInsecureFallback(false);
     job->setKey(kck);
-
     connect(job, SIGNAL(finished(QKeychain::Job *)), SLOT(slotReadClientKeyPEMJobDone(QKeychain::Job *)));
     job->start();
 }
@@ -236,12 +263,15 @@ void HttpCredentials::slotReadClientKeyPEMJobDone(QKeychain::Job *incoming)
     }
 
     // Now fetch the actual server password
-    const QString kck = keychainKey(_account->url().toString(), _user);
+    const QString kck = keychainKey(
+        _account->url().toString(),
+        _user,
+        _keychainMigration ? QString() : _account->id());
+
     ReadPasswordJob *job = new ReadPasswordJob(Theme::instance()->appName());
     addSettingsToJob(_account, job);
     job->setInsecureFallback(false);
     job->setKey(kck);
-
     connect(job, SIGNAL(finished(QKeychain::Job *)), SLOT(slotReadJobDone(QKeychain::Job *)));
     job->start();
 }
@@ -258,6 +288,17 @@ bool HttpCredentials::stillValid(QNetworkReply *reply)
 void HttpCredentials::slotReadJobDone(QKeychain::Job *incomingJob)
 {
     QKeychain::ReadPasswordJob *job = static_cast<ReadPasswordJob *>(incomingJob);
+    QKeychain::Error error = job->error();
+
+    // If we can't find the credentials at the keys that include the account id,
+    // try to read them from the legacy locations that don't have a account id.
+    if (!_keychainMigration && error == QKeychain::EntryNotFound) {
+        qCWarning(lcHttpCredentials)
+            << "Could not find keychain entries, attempting to read from legacy locations";
+        _keychainMigration = true;
+        fetchFromKeychainHelper();
+        return;
+    }
 
     bool isOauth = _account->credentialSetting(QLatin1String(isOAuthC)).toBool();
     if (isOauth) {
@@ -270,8 +311,6 @@ void HttpCredentials::slotReadJobDone(QKeychain::Job *incomingJob)
         qCWarning(lcHttpCredentials) << "Strange: User is empty!";
     }
 
-    QKeychain::Error error = job->error();
-
     if (!_refreshToken.isEmpty() && error == NoError) {
         refreshAccessToken();
     } else if (!_password.isEmpty() && error == NoError) {
@@ -290,6 +329,13 @@ void HttpCredentials::slotReadJobDone(QKeychain::Job *incomingJob)
         _ready = false;
         emit fetched();
     }
+
+    // If keychain data was read from legacy location, wipe these entries and store new ones
+    if (_keychainMigration && _ready) {
+        persist();
+        deleteOldKeychainEntries();
+        qCWarning(lcHttpCredentials) << "Migrated old keychain entries";
+    }
 }
 
 bool HttpCredentials::refreshAccessToken()
@@ -345,7 +391,7 @@ void HttpCredentials::invalidateToken()
     // User must be fetched from config file to generate a valid key
     fetchUser();
 
-    const QString kck = keychainKey(_account->url().toString(), _user);
+    const QString kck = keychainKey(_account->url().toString(), _user, _account->id());
     if (kck.isEmpty()) {
         qCWarning(lcHttpCredentials) << "InvalidateToken: User is empty, bailing out!";
         return;
@@ -407,7 +453,7 @@ void HttpCredentials::persist()
     addSettingsToJob(_account, job);
     job->setInsecureFallback(false);
     connect(job, SIGNAL(finished(QKeychain::Job *)), SLOT(slotWriteClientCertPEMJobDone(QKeychain::Job *)));
-    job->setKey(keychainKey(_account->url().toString(), _user + clientCertificatePEMC));
+    job->setKey(keychainKey(_account->url().toString(), _user + clientCertificatePEMC, _account->id()));
     job->setBinaryData(_clientSslCertificate.toPem());
     job->start();
 }
@@ -420,7 +466,7 @@ void HttpCredentials::slotWriteClientCertPEMJobDone(Job *incomingJob)
     addSettingsToJob(_account, job);
     job->setInsecureFallback(false);
     connect(job, SIGNAL(finished(QKeychain::Job *)), SLOT(slotWriteClientKeyPEMJobDone(QKeychain::Job *)));
-    job->setKey(keychainKey(_account->url().toString(), _user + clientKeyPEMC));
+    job->setKey(keychainKey(_account->url().toString(), _user + clientKeyPEMC, _account->id()));
     job->setBinaryData(_clientSslKey.toPem());
     job->start();
 }
@@ -432,7 +478,7 @@ void HttpCredentials::slotWriteClientKeyPEMJobDone(Job *incomingJob)
     addSettingsToJob(_account, job);
     job->setInsecureFallback(false);
     connect(job, SIGNAL(finished(QKeychain::Job *)), SLOT(slotWriteJobDone(QKeychain::Job *)));
-    job->setKey(keychainKey(_account->url().toString(), _user));
+    job->setKey(keychainKey(_account->url().toString(), _user, _account->id()));
     job->setTextData(isUsingOAuth() ? _refreshToken : _password);
     job->start();
 }

src/libsync/creds/httpcredentials.h

@@ -119,6 +119,18 @@ private Q_SLOTS:
     void slotWriteJobDone(QKeychain::Job *);
 
 protected:
+    /** Reads data from keychain locations
+     *
+     * Goes through
+     *   slotReadClientCertPEMJobDone to
+     *   slotReadClientCertPEMJobDone to
+     *   slotReadJobDone
+     */
+    void fetchFromKeychainHelper();
+
+    /// Wipes legacy keychain locations
+    void deleteOldKeychainEntries();
+
     QString _user;
     QString _password; // user's password, or access_token for OAuth
     QString _refreshToken; // OAuth _refreshToken, set if OAuth is used.
@@ -128,6 +140,7 @@ protected:
     bool _ready;
     QSslKey _clientSslKey;
     QSslCertificate _clientSslCertificate;
+    bool _keychainMigration;
 };