Skip to content

Commit

Permalink
Credentials: Use per-account keychain entries #5830
Browse files Browse the repository at this point in the history 
This requires a lot of migration code: the old entries need to be read,
saved to the new locations and then deleted.
  • Loading branch information
@ckamm
ckamm committed Sep 12, 2017
1 parent 70aafd7 commit 0719267
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 31 deletions.
50 changes: 41 additions & 9 deletions src/gui/creds/shibbolethcredentials.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ ShibbolethCredentials::ShibbolethCredentials()
, _ready(false)
, _stillValid(false)
, _browser(0)
, _keychainMigration(false)
{
}

Expand All @@ -62,6 +63,7 @@ ShibbolethCredentials::ShibbolethCredentials(const QNetworkCookie &cookie)
, _stillValid(true)
, _browser(0)
, _shibCookie(cookie)
, _keychainMigration(false)
{
}

Expand Down Expand Up @@ -131,15 +133,22 @@ void ShibbolethCredentials::fetchFromKeychain()
Q_EMIT fetched();
} else {
_url = _account->url();
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()));
connect(job, SIGNAL(finished(QKeychain::Job *)), SLOT(slotReadJobDone(QKeychain::Job *)));
job->start();
_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(_url.toString(), user(),
_keychainMigration ? QString() : _account->id()));
connect(job, SIGNAL(finished(QKeychain::Job *)), SLOT(slotReadJobDone(QKeychain::Job *)));
job->start();
}

void ShibbolethCredentials::askFromUser()
{
showLoginWindow();
Expand Down Expand Up @@ -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();
Expand All @@ -260,6 +279,20 @@ 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->setInsecureFallback(true);
job->setKey(keychainKey(_account->url().toString(), user(), QString()));
job->start();

qCWarning(lcShibboleth) << "Migrated old keychain entries";
}
}

void ShibbolethCredentials::showLoginWindow()
Expand Down Expand Up @@ -313,7 +346,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();
}
Expand All @@ -322,7 +355,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();
}

Expand All @@ -336,5 +369,4 @@ void ShibbolethCredentials::addToCookieJar(const QNetworkCookie &cookie)
jar->blockSignals(false);
}


} // namespace OCC
5 changes: 5 additions & 0 deletions src/gui/creds/shibbolethcredentials.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ private Q_SLOTS:
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;

Expand All @@ -92,6 +96,7 @@ private Q_SLOTS:
QPointer<ShibbolethWebView> _browser;
QNetworkCookie _shibCookie;
QString _user;
bool _keychainMigration;
};

} // namespace OCC
Expand Down
5 changes: 4 additions & 1 deletion src/libsync/creds/abstractcredentials.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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
2 changes: 1 addition & 1 deletion src/libsync/creds/abstractcredentials.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class OWNCLOUDSYNC_EXPORT AbstractCredentials : public QObject
*/
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.
Expand Down
86 changes: 66 additions & 20 deletions src/libsync/creds/httpcredentials.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ static void addSettingsToJob(Account *account, QKeychain::Job *job)

HttpCredentials::HttpCredentials()
: _ready(false)
, _keychainMigration(false)
{
}

Expand All @@ -113,6 +114,7 @@ HttpCredentials::HttpCredentials(const QString &user, const QString &password, c
, _ready(true)
, _clientSslKey(key)
, _clientSslCertificate(certificate)
, _keychainMigration(false)
{
}

Expand Down Expand Up @@ -174,21 +176,43 @@ void HttpCredentials::fetchFromKeychain()
return;
}

const QString kck = keychainKey(_account->url().toString(), _user);

if (_ready) {
Q_EMIT fetched();
} else {
// Read client cert from keychain
const QString kck = keychainKey(_account->url().toString(), _user + clientCertificatePEMC);
ReadPasswordJob *job = new ReadPasswordJob(Theme::instance()->appName());
addSettingsToJob(_account, job);
job->setInsecureFallback(false);
job->setKey(kck);
_keychainMigration = false;
fetchFromKeychainHelper();
}
}

connect(job, SIGNAL(finished(QKeychain::Job *)), SLOT(slotReadClientCertPEMJobDone(QKeychain::Job *)));
void HttpCredentials::fetchFromKeychainHelper()
{
// Read client cert from keychain
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)
Expand All @@ -203,12 +227,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();
}
Expand Down Expand Up @@ -238,12 +265,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();
}
Expand All @@ -260,6 +290,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) {
Expand All @@ -272,8 +313,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) {
Expand All @@ -292,6 +331,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()
Expand Down Expand Up @@ -344,7 +390,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;
Expand Down Expand Up @@ -406,7 +452,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();
}
Expand All @@ -419,7 +465,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();
}
Expand All @@ -431,7 +477,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();
}
Expand Down
13 changes: 13 additions & 0 deletions src/libsync/creds/httpcredentials.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -128,6 +140,7 @@ private Q_SLOTS:
bool _ready;
QSslKey _clientSslKey;
QSslCertificate _clientSslCertificate;
bool _keychainMigration;
};


Expand Down

0 comments on commit 0719267

Please sign in to comment.