Cleanup auto pointers and qobject casts, refactor KeychainChunk

Signed-off-by: default avatarMichael Schuster <michael@schuster.ms>
(cherry picked from commit f4d83d02f6ed62947e00879a0100ce778030815c)
Signed-off-by: default avatarMichael Schuster <michael@schuster.ms>
parent 6e2e1b36
......@@ -86,10 +86,6 @@ static void addSettingsToJob(Account *account, QKeychain::Job *job)
#endif
WebFlowCredentials::WebFlowCredentials()
: _ready(false)
, _credentialsValid(false)
, _keychainMigration(false)
, _retryOnKeyChainError(false)
{
}
......@@ -102,8 +98,6 @@ WebFlowCredentials::WebFlowCredentials(const QString &user, const QString &passw
, _clientSslCaCertificates(caCertificates)
, _ready(true)
, _credentialsValid(true)
, _keychainMigration(false)
, _retryOnKeyChainError(false)
{
}
......@@ -151,7 +145,7 @@ void WebFlowCredentials::fetchFromKeychain() {
void WebFlowCredentials::askFromUser() {
// Determine if the old flow has to be used (GS for now)
// Do a DetermineAuthTypeJob to make sure that the server is still using Flow2
DetermineAuthTypeJob *job = new DetermineAuthTypeJob(_account->sharedFromThis(), this);
auto job = new DetermineAuthTypeJob(_account->sharedFromThis(), this);
connect(job, &DetermineAuthTypeJob::authType, [this](DetermineAuthTypeJob::AuthType type) {
// LoginFlowV2 > WebViewFlow > OAuth > Shib > Basic
bool useFlow2 = (type != DetermineAuthTypeJob::WebViewFlow);
......@@ -244,10 +238,10 @@ void WebFlowCredentials::persist() {
// write cert if there is one
if (!_clientSslCertificate.isNull()) {
auto *job = new KeychainChunk::WriteJob(_account,
_user + clientCertificatePEMC,
_clientSslCertificate.toPem(),
this);
auto job = new KeychainChunk::WriteJob(_account,
_user + clientCertificatePEMC,
_clientSslCertificate.toPem(),
this);
connect(job, &KeychainChunk::WriteJob::finished, this, &WebFlowCredentials::slotWriteClientCertPEMJobDone);
job->start();
} else {
......@@ -260,10 +254,10 @@ void WebFlowCredentials::slotWriteClientCertPEMJobDone(KeychainChunk::WriteJob *
{
// write ssl key if there is one
if (!_clientSslKey.isNull()) {
auto *job = new KeychainChunk::WriteJob(_account,
_user + clientKeyPEMC,
_clientSslKey.toPem(),
this);
auto job = new KeychainChunk::WriteJob(_account,
_user + clientKeyPEMC,
_clientSslKey.toPem(),
this);
connect(job, &KeychainChunk::WriteJob::finished, this, &WebFlowCredentials::slotWriteClientKeyPEMJobDone);
job->start();
} else {
......@@ -291,10 +285,10 @@ void WebFlowCredentials::writeSingleClientCaCertPEM()
return;
}
auto *job = new KeychainChunk::WriteJob(_account,
_user + clientCaCertificatePEMC + QString::number(index),
cert.toPem(),
this);
auto job = new KeychainChunk::WriteJob(_account,
_user + clientCaCertificatePEMC + QString::number(index),
cert.toPem(),
this);
connect(job, &KeychainChunk::WriteJob::finished, this, &WebFlowCredentials::slotWriteClientCaCertsPEMJobDone);
job->start();
} else {
......@@ -334,7 +328,7 @@ void WebFlowCredentials::slotWriteClientCaCertsPEMJobDone(KeychainChunk::WriteJo
}
// done storing ca certs, time for the password
auto *job = new WritePasswordJob(Theme::instance()->appName(), this);
auto job = new WritePasswordJob(Theme::instance()->appName(), this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
#endif
......@@ -382,7 +376,7 @@ void WebFlowCredentials::forgetSensitiveData() {
return;
}
auto *job = new DeletePasswordJob(Theme::instance()->appName(), this);
auto job = new DeletePasswordJob(Theme::instance()->appName(), this);
job->setInsecureFallback(false);
job->setKey(kck);
job->start();
......@@ -435,10 +429,10 @@ void WebFlowCredentials::slotFinished(QNetworkReply *reply) {
void WebFlowCredentials::fetchFromKeychainHelper() {
// Read client cert from keychain
auto *job = new KeychainChunk::ReadJob(_account,
_user + clientCertificatePEMC,
_keychainMigration,
this);
auto job = new KeychainChunk::ReadJob(_account,
_user + clientCertificatePEMC,
_keychainMigration,
this);
connect(job, &KeychainChunk::ReadJob::finished, this, &WebFlowCredentials::slotReadClientCertPEMJobDone);
job->start();
}
......@@ -454,10 +448,10 @@ void WebFlowCredentials::slotReadClientCertPEMJobDone(KeychainChunk::ReadJob *re
}
// Load key too
auto *job = new KeychainChunk::ReadJob(_account,
_user + clientKeyPEMC,
_keychainMigration,
this);
auto job = new KeychainChunk::ReadJob(_account,
_user + clientKeyPEMC,
_keychainMigration,
this);
connect(job, &KeychainChunk::ReadJob::finished, this, &WebFlowCredentials::slotReadClientKeyPEMJobDone);
job->start();
}
......@@ -494,10 +488,10 @@ void WebFlowCredentials::readSingleClientCaCertPEM()
{
// try to fetch a client ca cert
if (_clientSslCaCertificates.count() < _clientSslCaCertificatesMaxCount) {
auto *job = new KeychainChunk::ReadJob(_account,
_user + clientCaCertificatePEMC + QString::number(_clientSslCaCertificates.count()),
_keychainMigration,
this);
auto job = new KeychainChunk::ReadJob(_account,
_user + clientCaCertificatePEMC + QString::number(_clientSslCaCertificates.count()),
_keychainMigration,
this);
connect(job, &KeychainChunk::ReadJob::finished, this, &WebFlowCredentials::slotReadClientCaCertsPEMJobDone);
job->start();
} else {
......@@ -533,7 +527,7 @@ void WebFlowCredentials::slotReadClientCaCertsPEMJobDone(KeychainChunk::ReadJob
_user,
_keychainMigration ? QString() : _account->id());
auto *job = new ReadPasswordJob(Theme::instance()->appName(), this);
auto job = new ReadPasswordJob(Theme::instance()->appName(), this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
#endif
......@@ -544,7 +538,7 @@ void WebFlowCredentials::slotReadClientCaCertsPEMJobDone(KeychainChunk::ReadJob
}
void WebFlowCredentials::slotReadPasswordJobDone(Job *incomingJob) {
QKeychain::ReadPasswordJob *job = qobject_cast<ReadPasswordJob *>(incomingJob);
auto job = qobject_cast<ReadPasswordJob *>(incomingJob);
QKeychain::Error error = job->error();
// If we could not find the entry try the old entries
......@@ -578,7 +572,7 @@ void WebFlowCredentials::slotReadPasswordJobDone(Job *incomingJob) {
void WebFlowCredentials::deleteKeychainEntries(bool oldKeychainEntries) {
auto startDeleteJob = [this, oldKeychainEntries](QString key) {
auto *job = new KeychainChunk::DeleteJob(_account, key, oldKeychainEntries, this);
auto job = new KeychainChunk::DeleteJob(_account, key, oldKeychainEntries, this);
job->start();
};
......
......@@ -35,10 +35,6 @@ ProxyAuthHandler *ProxyAuthHandler::instance()
}
ProxyAuthHandler::ProxyAuthHandler()
: _blocked(false)
, _waitingForDialog(0)
, _waitingForKeychain(0)
, _keychainJobRunning(false)
{
_dialog = new ProxyAuthDialog();
......@@ -86,7 +82,7 @@ void ProxyAuthHandler::handleProxyAuthenticationRequired(
// Find the responsible QNAM if possible.
QNetworkAccessManager *sending_qnam = nullptr;
QWeakPointer<QNetworkAccessManager> qnam_alive;
if (Account *account = qobject_cast<Account *>(sender())) {
if (auto account = qobject_cast<Account *>(sender())) {
// Since we go into an event loop, it's possible for the account's qnam
// to be destroyed before we get back. We can use this to check for its
// liveness.
......@@ -176,8 +172,9 @@ void ProxyAuthHandler::execAwait(const T *sender,
int &counter,
const QEventLoop::ProcessEventsFlags flags)
{
if(!sender)
if (!sender) {
return;
}
QEventLoop waitLoop;
connect(sender, signal, &waitLoop, &QEventLoop::quit);
......@@ -240,7 +237,7 @@ void ProxyAuthHandler::storeCredsInKeychain()
_settings->setValue(keychainUsernameKey(), _username);
WritePasswordJob *job = new WritePasswordJob(Theme::instance()->appName(), this);
auto job = new WritePasswordJob(Theme::instance()->appName(), this);
job->setSettings(_settings.data());
job->setInsecureFallback(false);
job->setKey(keychainPasswordKey());
......
......@@ -23,6 +23,7 @@
#include <QScopedPointer>
#include <QSettings>
#include <QSet>
#include <QEventLoop>
namespace QKeychain {
class Job;
......@@ -89,7 +90,7 @@ private:
/// If the user cancels the credential dialog, blocked will be set to
/// true and we won't bother him again.
bool _blocked;
bool _blocked = false;
/// In several instances handleProxyAuthenticationRequired() can be called
/// while it is still running. These counters detect what we're currently
......
......@@ -658,11 +658,11 @@ void ConfigFile::setProxyType(int proxyType,
settings.remove(QLatin1String(proxyPassC));
// Delete password from keychain
auto *job = new KeychainChunk::DeleteJob(keychainProxyPasswordKey());
auto job = new KeychainChunk::DeleteJob(keychainProxyPasswordKey());
job->exec();
} else {
// Write password to keychain
auto *job = new KeychainChunk::WriteJob(keychainProxyPasswordKey(), pass.toUtf8());
auto job = new KeychainChunk::WriteJob(keychainProxyPasswordKey(), pass.toUtf8());
if (job->exec()) {
// Security: Don't keep password in config file
settings.remove(QLatin1String(proxyPassC));
......@@ -750,7 +750,7 @@ QString ConfigFile::proxyPassword() const
if (!pass.isEmpty()) {
// Security: Migrate password from config file to keychain
auto *job = new KeychainChunk::WriteJob(key, pass.toUtf8());
auto job = new KeychainChunk::WriteJob(key, pass.toUtf8());
if (job->exec()) {
QSettings settings(configFile(), QSettings::IniFormat);
settings.remove(QLatin1String(proxyPassC));
......@@ -758,7 +758,7 @@ QString ConfigFile::proxyPassword() const
}
} else {
// Read password from keychain
auto *job = new KeychainChunk::ReadJob(key);
auto job = new KeychainChunk::ReadJob(key);
if (job->exec()) {
pass = job->textData();
}
......
......@@ -70,6 +70,48 @@ Job::~Job()
_chunkBuffer.clear();
}
QKeychain::Error Job::error() const
{
return _error;
}
QString Job::errorString() const
{
return _errorString;
}
QByteArray Job::binaryData() const
{
return _chunkBuffer;
}
QString Job::textData() const
{
return _chunkBuffer;
}
bool Job::insecureFallback() const
{
return _insecureFallback;
}
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
void Job::setInsecureFallback(bool insecureFallback)
{
_insecureFallback = insecureFallback;
}
#endif
bool Job::autoDelete() const
{
return _autoDelete;
}
void Job::setAutoDelete(bool autoDelete)
{
_autoDelete = autoDelete;
}
/*
* WriteJob
*/
......@@ -118,9 +160,9 @@ bool WriteJob::exec()
void WriteJob::slotWriteJobDone(QKeychain::Job *incomingJob)
{
QKeychain::WritePasswordJob *writeJob = static_cast<QKeychain::WritePasswordJob *>(incomingJob);
auto writeJob = qobject_cast<QKeychain::WritePasswordJob *>(incomingJob);
// errors?
// Errors? (writeJob can be nullptr here, see: WriteJob::start)
if (writeJob) {
_error = writeJob->error();
_errorString = writeJob->errorString();
......@@ -157,8 +199,9 @@ void WriteJob::slotWriteJobDone(QKeychain::Job *incomingJob)
emit finished(this);
if (_autoDelete)
if (_autoDelete) {
deleteLater();
}
return;
}
......@@ -169,7 +212,7 @@ void WriteJob::slotWriteJobDone(QKeychain::Job *incomingJob)
_account->id()
) : keyWithIndex;
auto *job = new QKeychain::WritePasswordJob(_serviceName, this);
auto job = new QKeychain::WritePasswordJob(_serviceName, this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
#endif
......@@ -184,8 +227,9 @@ void WriteJob::slotWriteJobDone(QKeychain::Job *incomingJob)
} else {
emit finished(this);
if (_autoDelete)
if (_autoDelete) {
deleteLater();
}
}
writeJob->deleteLater();
......@@ -194,7 +238,7 @@ void WriteJob::slotWriteJobDone(QKeychain::Job *incomingJob)
/*
* ReadJob
*/
ReadJob::ReadJob(Account *account, const QString &key, const bool &keychainMigration, QObject *parent)
ReadJob::ReadJob(Account *account, const QString &key, bool keychainMigration, QObject *parent)
: Job(parent)
{
_account = account;
......@@ -226,7 +270,7 @@ void ReadJob::start()
_keychainMigration ? QString() : _account->id()
) : _key;
auto *job = new QKeychain::ReadPasswordJob(_serviceName, this);
auto job = new QKeychain::ReadPasswordJob(_serviceName, this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
#endif
......@@ -259,77 +303,77 @@ bool ReadJob::exec()
void ReadJob::slotReadJobDone(QKeychain::Job *incomingJob)
{
// Errors or next chunk?
QKeychain::ReadPasswordJob *readJob = static_cast<QKeychain::ReadPasswordJob *>(incomingJob);
auto readJob = qobject_cast<QKeychain::ReadPasswordJob *>(incomingJob);
Q_ASSERT(readJob);
if (readJob) {
if (readJob->error() == NoError && readJob->binaryData().length() > 0) {
_chunkBuffer.append(readJob->binaryData());
_chunkCount++;
if (readJob->error() == NoError && !readJob->binaryData().isEmpty()) {
_chunkBuffer.append(readJob->binaryData());
_chunkCount++;
#if defined(Q_OS_WIN)
// try to fetch next chunk
if (_chunkCount < KeychainChunk::MaxChunks) {
const QString keyWithIndex = _key + QString(".") + QString::number(_chunkCount);
const QString kck = _account ? AbstractCredentials::keychainKey(
_account->url().toString(),
keyWithIndex,
_keychainMigration ? QString() : _account->id()
) : keyWithIndex;
QKeychain::ReadPasswordJob *job = new QKeychain::ReadPasswordJob(_serviceName, this);
// try to fetch next chunk
if (_chunkCount < KeychainChunk::MaxChunks) {
const QString keyWithIndex = _key + QString(".") + QString::number(_chunkCount);
const QString kck = _account ? AbstractCredentials::keychainKey(
_account->url().toString(),
keyWithIndex,
_keychainMigration ? QString() : _account->id()
) : keyWithIndex;
auto job = new QKeychain::ReadPasswordJob(_serviceName, this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
addSettingsToJob(_account, job);
#endif
job->setInsecureFallback(_insecureFallback);
job->setKey(kck);
connect(job, &QKeychain::Job::finished, this, &KeychainChunk::ReadJob::slotReadJobDone);
job->start();
job->setInsecureFallback(_insecureFallback);
job->setKey(kck);
connect(job, &QKeychain::Job::finished, this, &KeychainChunk::ReadJob::slotReadJobDone);
job->start();
readJob->deleteLater();
return;
} else {
qCWarning(lcKeychainChunk) << "Maximum chunk count for" << readJob->key() << "reached, ignoring after" << KeychainChunk::MaxChunks;
}
#endif
readJob->deleteLater();
return;
} else {
qCWarning(lcKeychainChunk) << "Maximum chunk count for" << readJob->key() << "reached, ignoring after" << KeychainChunk::MaxChunks;
}
#endif
} else {
#if defined(Q_OS_UNIX) && !defined(Q_OS_MAC)
if (!readJob->insecureFallback()) { // If insecureFallback is set, the next test would be pointless
if (_retryOnKeyChainError && (readJob->error() == QKeychain::NoBackendAvailable
|| readJob->error() == QKeychain::OtherError)) {
// Could be that the backend was not yet available. Wait some extra seconds.
// (Issues #4274 and #6522)
// (For kwallet, the error is OtherError instead of NoBackendAvailable, maybe a bug in QtKeychain)
qCInfo(lcKeychainChunk) << "Backend unavailable (yet?) Retrying in a few seconds." << readJob->errorString();
QTimer::singleShot(10000, this, &ReadJob::start);
_retryOnKeyChainError = false;
readJob->deleteLater();
return;
}
if (!readJob->insecureFallback()) { // If insecureFallback is set, the next test would be pointless
if (_retryOnKeyChainError && (readJob->error() == QKeychain::NoBackendAvailable
|| readJob->error() == QKeychain::OtherError)) {
// Could be that the backend was not yet available. Wait some extra seconds.
// (Issues #4274 and #6522)
// (For kwallet, the error is OtherError instead of NoBackendAvailable, maybe a bug in QtKeychain)
qCInfo(lcKeychainChunk) << "Backend unavailable (yet?) Retrying in a few seconds." << readJob->errorString();
QTimer::singleShot(10000, this, &ReadJob::start);
_retryOnKeyChainError = false;
readJob->deleteLater();
return;
}
_retryOnKeyChainError = false;
}
#endif
if (readJob->error() != QKeychain::EntryNotFound ||
((readJob->error() == QKeychain::EntryNotFound) && _chunkCount == 0)) {
_error = readJob->error();
_errorString = readJob->errorString();
qCWarning(lcKeychainChunk) << "Unable to read" << readJob->key() << "chunk" << QString::number(_chunkCount) << readJob->errorString();
}
if (readJob->error() != QKeychain::EntryNotFound ||
((readJob->error() == QKeychain::EntryNotFound) && _chunkCount == 0)) {
_error = readJob->error();
_errorString = readJob->errorString();
qCWarning(lcKeychainChunk) << "Unable to read" << readJob->key() << "chunk" << QString::number(_chunkCount) << readJob->errorString();
}
readJob->deleteLater();
}
readJob->deleteLater();
emit finished(this);
if (_autoDelete)
if (_autoDelete) {
deleteLater();
}
}
/*
* DeleteJob
*/
DeleteJob::DeleteJob(Account *account, const QString &key, const bool &keychainMigration, QObject *parent)
DeleteJob::DeleteJob(Account *account, const QString &key, bool keychainMigration, QObject *parent)
: Job(parent)
{
_account = account;
......@@ -357,7 +401,7 @@ void DeleteJob::start()
_keychainMigration ? QString() : _account->id()
) : _key;
auto *job = new QKeychain::DeletePasswordJob(_serviceName, this);
auto job = new QKeychain::DeletePasswordJob(_serviceName, this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
#endif
......@@ -389,53 +433,53 @@ bool DeleteJob::exec()
void DeleteJob::slotDeleteJobDone(QKeychain::Job *incomingJob)
{
// Errors or next chunk?
auto *deleteJob = static_cast<QKeychain::DeletePasswordJob *>(incomingJob);
auto deleteJob = qobject_cast<QKeychain::DeletePasswordJob *>(incomingJob);
Q_ASSERT(deleteJob);
if (deleteJob) {
if (deleteJob->error() == NoError) {
_chunkCount++;
if (deleteJob->error() == NoError) {
_chunkCount++;
#if defined(Q_OS_WIN)
// try to delete next chunk
if (_chunkCount < KeychainChunk::MaxChunks) {
const QString keyWithIndex = _key + QString(".") + QString::number(_chunkCount);
const QString kck = _account ? AbstractCredentials::keychainKey(
_account->url().toString(),
keyWithIndex,
_keychainMigration ? QString() : _account->id()
) : keyWithIndex;
QKeychain::DeletePasswordJob *job = new QKeychain::DeletePasswordJob(_serviceName, this);
// try to delete next chunk
if (_chunkCount < KeychainChunk::MaxChunks) {
const QString keyWithIndex = _key + QString(".") + QString::number(_chunkCount);
const QString kck = _account ? AbstractCredentials::keychainKey(
_account->url().toString(),
keyWithIndex,
_keychainMigration ? QString() : _account->id()
) : keyWithIndex;
auto job = new QKeychain::DeletePasswordJob(_serviceName, this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
addSettingsToJob(_account, job);
#endif
job->setInsecureFallback(_insecureFallback);
job->setKey(kck);
connect(job, &QKeychain::Job::finished, this, &KeychainChunk::DeleteJob::slotDeleteJobDone);
job->start();
job->setInsecureFallback(_insecureFallback);
job->setKey(kck);
connect(job, &QKeychain::Job::finished, this, &KeychainChunk::DeleteJob::slotDeleteJobDone);
job->start();
deleteJob->deleteLater();
return;
} else {
qCWarning(lcKeychainChunk) << "Maximum chunk count for" << deleteJob->key() << "reached, ignoring after" << KeychainChunk::MaxChunks;
}
#endif
deleteJob->deleteLater();
return;
} else {
if (deleteJob->error() != QKeychain::EntryNotFound ||
((deleteJob->error() == QKeychain::EntryNotFound) && _chunkCount == 0)) {
_error = deleteJob->error();
_errorString = deleteJob->errorString();
qCWarning(lcKeychainChunk) << "Unable to delete" << deleteJob->key() << "chunk" << QString::number(_chunkCount) << deleteJob->errorString();
}
qCWarning(lcKeychainChunk) << "Maximum chunk count for" << deleteJob->key() << "reached, ignoring after" << KeychainChunk::MaxChunks;
}
#endif
} else {
if (deleteJob->error() != QKeychain::EntryNotFound ||
((deleteJob->error() == QKeychain::EntryNotFound) && _chunkCount == 0)) {
_error = deleteJob->error();
_errorString = deleteJob->errorString();
qCWarning(lcKeychainChunk) << "Unable to delete" << deleteJob->key() << "chunk" << QString::number(_chunkCount) << deleteJob->errorString();
}
deleteJob->deleteLater();
}
deleteJob->deleteLater();
emit finished(this);
if (_autoDelete)
if (_autoDelete) {
deleteLater();
}
}
} // namespace KeychainChunk
......
......@@ -47,47 +47,30 @@ public:
virtual ~Job();
const QKeychain::Error error() const {
return _error;
}
const QString errorString() const {
return _errorString;
}
QByteArray binaryData() const {
return _chunkBuffer;
}
QString textData() const {
return _chunkBuffer;
}
const bool insecureFallback() const {
return _insecureFallback;
}
QKeychain::Error error() const;
QString errorString() const;
QByteArray binaryData() const;
QString textData() const;
bool insecureFallback() const;