Skip to content

Commit

Permalink
Const auto-fy all references to QtKeychain jobs
Browse files Browse the repository at this point in the history 
Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>
  • Loading branch information
@claucambra
claucambra committed Feb 9, 2023
1 parent 6f72ce0 commit d7cd797
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 45 deletions.
2 changes: 1 addition & 1 deletion src/gui/creds/webflowcredentials.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ void WebFlowCredentials::slotWritePassword()
// Since QtKeychain does not (yet) run this operation on a separate thread,
// we run this with QtConcurrent to prevent a full application freeze.
QtConcurrent::run([keychainEntryKey, this] {
auto job = new WritePasswordJob(Theme::instance()->appName());
const auto job = new WritePasswordJob(Theme::instance()->appName());
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
#endif
Expand Down
12 changes: 6 additions & 6 deletions src/libsync/account.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -753,13 +753,13 @@ void Account::writeAppPasswordOnce(QString appPassword){
// Since QtKeychain does not (yet) run this operation on a separate thread,
// we run this with QtConcurrent to prevent a full application freeze.
QtConcurrent::run([kck, appPassword, this] {
auto *job = new WritePasswordJob(Theme::instance()->appName());
const auto job = new WritePasswordJob(Theme::instance()->appName());
job->setInsecureFallback(false);
job->setKey(kck);
job->setBinaryData(appPassword.toLatin1());

connect(job, &WritePasswordJob::finished, this, [this](Job *incoming) {
auto *writeJob = dynamic_cast<WritePasswordJob *>(incoming);
const auto writeJob = dynamic_cast<WritePasswordJob *>(incoming);
if (writeJob->error() == NoError) {
qCInfo(lcAccount) << "appPassword stored in keychain";
} else {
Expand All @@ -781,11 +781,11 @@ void Account::retrieveAppPassword(){
id()
);

auto *job = new ReadPasswordJob(Theme::instance()->appName());
const auto job = new ReadPasswordJob(Theme::instance()->appName());
job->setInsecureFallback(false);
job->setKey(kck);
connect(job, &ReadPasswordJob::finished, [this](Job *incoming) {
auto *readJob = dynamic_cast<ReadPasswordJob *>(incoming);
const auto readJob = dynamic_cast<ReadPasswordJob *>(incoming);
QString pwd("");
// Error or no valid public key error out
if (readJob->error() == NoError &&
Expand All @@ -811,11 +811,11 @@ void Account::deleteAppPassword()
return;
}

auto *job = new DeletePasswordJob(Theme::instance()->appName());
const auto job = new DeletePasswordJob(Theme::instance()->appName());
job->setInsecureFallback(false);
job->setKey(kck);
connect(job, &DeletePasswordJob::finished, [this](Job *incoming) {
auto *deleteJob = dynamic_cast<DeletePasswordJob *>(incoming);
const auto deleteJob = dynamic_cast<DeletePasswordJob *>(incoming);
const auto jobError = deleteJob->error();
if (jobError == NoError) {
qCInfo(lcAccount) << "appPassword deleted from keychain";
Expand Down
8 changes: 4 additions & 4 deletions src/libsync/clientsideencryption.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ void ClientSideEncryption::writePrivateKey(const AccountPtr &account)
// Since QtKeychain does not (yet) run this operation on a separate thread,
// we run this with QtConcurrent to prevent a full application freeze.
QtConcurrent::run([kck, this] {
auto *job = new WritePasswordJob(Theme::instance()->appName());
const auto job = new WritePasswordJob(Theme::instance()->appName());
job->setInsecureFallback(false);
job->setKey(kck);
job->setBinaryData(_privateKey);
Expand All @@ -1078,7 +1078,7 @@ void ClientSideEncryption::writeCertificate(const AccountPtr &account)
// Since QtKeychain does not (yet) run this operation on a separate thread,
// we run this with QtConcurrent to prevent a full application freeze.
QtConcurrent::run([kck, this] {
auto *job = new WritePasswordJob(Theme::instance()->appName());
const auto job = new WritePasswordJob(Theme::instance()->appName());
job->setInsecureFallback(false);
job->setKey(kck);
job->setBinaryData(_certificate.toPem());
Expand All @@ -1102,7 +1102,7 @@ void ClientSideEncryption::writeMnemonic(const AccountPtr &account)
// Since QtKeychain does not (yet) run this operation on a separate thread,
// we run this with QtConcurrent to prevent a full application freeze.
QtConcurrent::run([kck, this] {
auto *job = new WritePasswordJob(Theme::instance()->appName());
const auto job = new WritePasswordJob(Theme::instance()->appName());
job->setInsecureFallback(false);
job->setKey(kck);
job->setTextData(_mnemonic);
Expand All @@ -1124,7 +1124,7 @@ void ClientSideEncryption::forgetSensitiveData(const AccountPtr &account)
}

const auto createDeleteJob = [account](const QString user) {
auto *job = new DeletePasswordJob(Theme::instance()->appName());
const auto job = new DeletePasswordJob(Theme::instance()->appName());
job->setInsecureFallback(false);
job->setKey(AbstractCredentials::keychainKey(account->url().toString(), user, account->id()));
return job;
Expand Down
53 changes: 29 additions & 24 deletions src/libsync/creds/httpcredentials.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ void HttpCredentials::fetchFromKeychainHelper()
if (!_clientCertBundle.isEmpty()) {
// New case (>=2.6): We have a bundle in the settings and read the password from
// the keychain
auto job = new QKeychain::ReadPasswordJob(Theme::instance()->appName());
const auto job = new QKeychain::ReadPasswordJob(Theme::instance()->appName());
addSettingsToJob(_account, job);
job->setInsecureFallback(false);
job->setKey(keychainKey(_account->url().toString(), _user + clientCertPasswordC, _account->id()));
Expand All @@ -210,12 +210,12 @@ void HttpCredentials::fetchFromKeychainHelper()
}

// Old case (pre 2.6): Read client cert and then key from keychain
const QString kck = keychainKey(
const auto kck = keychainKey(
_account->url().toString(),
_user + clientCertificatePEMC,
_keychainMigration ? QString() : _account->id());

auto *job = new QKeychain::ReadPasswordJob(Theme::instance()->appName());
const auto job = new QKeychain::ReadPasswordJob(Theme::instance()->appName());
addSettingsToJob(_account, job);
job->setInsecureFallback(false);
job->setKey(kck);
Expand All @@ -225,8 +225,8 @@ void HttpCredentials::fetchFromKeychainHelper()

void HttpCredentials::deleteOldKeychainEntries()
{
auto startDeleteJob = [this](QString user) {
auto *job = new QKeychain::DeletePasswordJob(Theme::instance()->appName());
const auto startDeleteJob = [this](QString user) {
const auto job = new QKeychain::DeletePasswordJob(Theme::instance()->appName());
addSettingsToJob(_account, job);
job->setInsecureFallback(true);
job->setKey(keychainKey(_account->url().toString(), user, QString()));
Expand Down Expand Up @@ -261,9 +261,10 @@ bool HttpCredentials::keychainUnavailableRetryLater(QKeychain::ReadPasswordJob *

void HttpCredentials::slotReadClientCertPasswordJobDone(QKeychain::Job *job)
{
auto readJob = qobject_cast<QKeychain::ReadPasswordJob*>(job);
if (keychainUnavailableRetryLater(readJob))
const auto readJob = qobject_cast<QKeychain::ReadPasswordJob*>(job);
if (keychainUnavailableRetryLater(readJob)) {
return;
}

if (readJob->error() == QKeychain::NoError) {
_clientCertPassword = readJob->binaryData();
Expand All @@ -282,9 +283,10 @@ void HttpCredentials::slotReadClientCertPasswordJobDone(QKeychain::Job *job)

void HttpCredentials::slotReadClientCertPEMJobDone(QKeychain::Job *incoming)
{
auto readJob = qobject_cast<QKeychain::ReadPasswordJob*>(incoming);
if (keychainUnavailableRetryLater(readJob))
const auto readJob = qobject_cast<QKeychain::ReadPasswordJob*>(incoming);
if (keychainUnavailableRetryLater(readJob)) {
return;
}

// Store PEM in memory
if (readJob->error() == QKeychain::NoError && readJob->binaryData().length() > 0) {
Expand All @@ -295,12 +297,12 @@ void HttpCredentials::slotReadClientCertPEMJobDone(QKeychain::Job *incoming)
}

// Load key too
const QString kck = keychainKey(
const auto kck = keychainKey(
_account->url().toString(),
_user + clientKeyPEMC,
_keychainMigration ? QString() : _account->id());

auto *job = new QKeychain::ReadPasswordJob(Theme::instance()->appName());
const auto job = new QKeychain::ReadPasswordJob(Theme::instance()->appName());
addSettingsToJob(_account, job);
job->setInsecureFallback(false);
job->setKey(kck);
Expand All @@ -310,7 +312,7 @@ void HttpCredentials::slotReadClientCertPEMJobDone(QKeychain::Job *incoming)

void HttpCredentials::slotReadClientKeyPEMJobDone(QKeychain::Job *incoming)
{
auto readJob = qobject_cast<QKeychain::ReadPasswordJob*>(incoming);
const auto readJob = qobject_cast<QKeychain::ReadPasswordJob*>(incoming);
// Store key in memory

if (readJob->error() == QKeychain::NoError && readJob->binaryData().length() > 0) {
Expand All @@ -334,12 +336,12 @@ void HttpCredentials::slotReadClientKeyPEMJobDone(QKeychain::Job *incoming)

void HttpCredentials::slotReadPasswordFromKeychain()
{
const QString kck = keychainKey(
const auto kck = keychainKey(
_account->url().toString(),
_user,
_keychainMigration ? QString() : _account->id());

auto *job = new QKeychain::ReadPasswordJob(Theme::instance()->appName());
const auto job = new QKeychain::ReadPasswordJob(Theme::instance()->appName());
addSettingsToJob(_account, job);
job->setInsecureFallback(false);
job->setKey(kck);
Expand All @@ -358,8 +360,8 @@ bool HttpCredentials::stillValid(QNetworkReply *reply)

void HttpCredentials::slotReadJobDone(QKeychain::Job *incoming)
{
auto *job = dynamic_cast<QKeychain::ReadPasswordJob *>(incoming);
QKeychain::Error error = job->error();
const auto job = dynamic_cast<QKeychain::ReadPasswordJob *>(incoming);
const auto 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.
Expand Down Expand Up @@ -487,7 +489,7 @@ void HttpCredentials::invalidateToken()
return;
}

auto *job = new QKeychain::DeletePasswordJob(Theme::instance()->appName());
const auto job = new QKeychain::DeletePasswordJob(Theme::instance()->appName());
addSettingsToJob(_account, job);
job->setInsecureFallback(true);
job->setKey(kck);
Expand Down Expand Up @@ -537,7 +539,7 @@ void HttpCredentials::persist()
// Since QtKeychain does not (yet) run this operation on a separate thread,
// we run this with QtConcurrent to prevent a full application freeze.
QtConcurrent::run([this] {
auto *job = new QKeychain::WritePasswordJob(Theme::instance()->appName());
const auto job = new QKeychain::WritePasswordJob(Theme::instance()->appName());
addSettingsToJob(_account, job);
job->setInsecureFallback(false);
connect(job, &QKeychain::Job::finished, this, &HttpCredentials::slotWriteClientCertPasswordJobDone);
Expand All @@ -554,7 +556,7 @@ void HttpCredentials::persist()
// because we have no functions for creating an encrypted pkcs12 bundle.

QtConcurrent::run([this] {
auto *job = new QKeychain::WritePasswordJob(Theme::instance()->appName());
const auto job = new QKeychain::WritePasswordJob(Theme::instance()->appName());
addSettingsToJob(_account, job);
job->setInsecureFallback(false);
connect(job, &QKeychain::Job::finished, this, &HttpCredentials::slotWriteClientCertPEMJobDone);
Expand Down Expand Up @@ -587,7 +589,7 @@ void HttpCredentials::slotWriteClientCertPEMJobDone(QKeychain::Job *finishedJob)

// write ssl key if there is one
if (!_clientSslKey.isNull()) {
auto *job = new QKeychain::WritePasswordJob(Theme::instance()->appName());
const auto job = new QKeychain::WritePasswordJob(Theme::instance()->appName());
addSettingsToJob(_account, job);
job->setInsecureFallback(false);
connect(job, &QKeychain::Job::finished, this, &HttpCredentials::slotWriteClientKeyPEMJobDone);
Expand All @@ -612,7 +614,7 @@ void HttpCredentials::slotWriteClientKeyPEMJobDone(QKeychain::Job *finishedJob)
void HttpCredentials::slotWritePasswordToKeychain()
{
QtConcurrent::run([this] {
auto *job = new QKeychain::WritePasswordJob(Theme::instance()->appName());
const auto job = new QKeychain::WritePasswordJob(Theme::instance()->appName());
addSettingsToJob(_account, job);
job->setInsecureFallback(false);
connect(job, &QKeychain::Job::finished, this, &HttpCredentials::slotWriteJobDone);
Expand All @@ -632,8 +634,10 @@ void HttpCredentials::slotWriteJobDone(QKeychain::Job *job)

void HttpCredentials::slotAuthentication(QNetworkReply *reply, QAuthenticator *authenticator)
{
if (!_ready)
if (!_ready) {
return;
}

Q_UNUSED(authenticator)
// Because of issue #4326, we need to set the login and password manually at every requests
// Thus, if we reach this signal, those credentials were invalid and we terminate.
Expand All @@ -651,7 +655,7 @@ void HttpCredentials::slotAuthentication(QNetworkReply *reply, QAuthenticator *a

bool HttpCredentials::retryIfNeeded(AbstractNetworkJob *job)
{
auto *reply = job->reply();
const auto reply = job->reply();
if (!reply || !reply->property(needRetryC).toBool())
return false;
if (_isRenewingOAuthToken) {
Expand All @@ -664,8 +668,9 @@ bool HttpCredentials::retryIfNeeded(AbstractNetworkJob *job)

bool HttpCredentials::unpackClientCertBundle()
{
if (_clientCertBundle.isEmpty())
if (_clientCertBundle.isEmpty()) {
return true;
}

QBuffer certBuffer(&_clientCertBundle);
certBuffer.open(QIODevice::ReadOnly);
Expand Down
20 changes: 10 additions & 10 deletions src/libsync/creds/keychainchunk.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ void WriteJob::slotWriteJobDone(QKeychain::Job *incomingJob)
// Since QtKeychain does not (yet) run this operation on a separate thread,
// we run this with QtConcurrent to prevent a full application freeze.
QtConcurrent::run([kck, chunk, this] {
auto job = new QKeychain::WritePasswordJob(_serviceName, this);
const auto job = new QKeychain::WritePasswordJob(_serviceName, this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
#endif
Expand Down Expand Up @@ -276,7 +276,7 @@ void ReadJob::start()
_keychainMigration ? QString() : _account->id()
) : _key;

auto job = new QKeychain::ReadPasswordJob(_serviceName, this);
const auto job = new QKeychain::ReadPasswordJob(_serviceName, this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
#endif
Expand Down Expand Up @@ -309,7 +309,7 @@ bool ReadJob::exec()
void ReadJob::slotReadJobDone(QKeychain::Job *incomingJob)
{
// Errors or next chunk?
auto readJob = qobject_cast<QKeychain::ReadPasswordJob *>(incomingJob);
const auto readJob = qobject_cast<QKeychain::ReadPasswordJob *>(incomingJob);
Q_ASSERT(readJob);

if (readJob->error() == NoError && !readJob->binaryData().isEmpty()) {
Expand All @@ -320,13 +320,13 @@ void ReadJob::slotReadJobDone(QKeychain::Job *incomingJob)
// try to fetch next chunk
if (_chunkCount < KeychainChunk::MaxChunks) {
const QString keyWithIndex = _key + QString(".") + QString::number(_chunkCount);
const QString kck = _account ? AbstractCredentials::keychainKey(
const auto kck = _account ? AbstractCredentials::keychainKey(
_account->url().toString(),
keyWithIndex,
_keychainMigration ? QString() : _account->id()
) : keyWithIndex;

auto job = new QKeychain::ReadPasswordJob(_serviceName, this);
const auto job = new QKeychain::ReadPasswordJob(_serviceName, this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
#endif
Expand Down Expand Up @@ -401,13 +401,13 @@ void DeleteJob::start()
_chunkCount = 0;
_error = QKeychain::NoError;

const QString kck = _account ? AbstractCredentials::keychainKey(
const auto kck = _account ? AbstractCredentials::keychainKey(
_account->url().toString(),
_key,
_keychainMigration ? QString() : _account->id()
) : _key;

auto job = new QKeychain::DeletePasswordJob(_serviceName, this);
const auto job = new QKeychain::DeletePasswordJob(_serviceName, this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
#endif
Expand Down Expand Up @@ -439,7 +439,7 @@ bool DeleteJob::exec()
void DeleteJob::slotDeleteJobDone(QKeychain::Job *incomingJob)
{
// Errors or next chunk?
auto deleteJob = qobject_cast<QKeychain::DeletePasswordJob *>(incomingJob);
const auto deleteJob = qobject_cast<QKeychain::DeletePasswordJob *>(incomingJob);
Q_ASSERT(deleteJob);

if (deleteJob->error() == NoError) {
Expand All @@ -449,13 +449,13 @@ void DeleteJob::slotDeleteJobDone(QKeychain::Job *incomingJob)
// try to delete next chunk
if (_chunkCount < KeychainChunk::MaxChunks) {
const QString keyWithIndex = _key + QString(".") + QString::number(_chunkCount);
const QString kck = _account ? AbstractCredentials::keychainKey(
const auto kck = _account ? AbstractCredentials::keychainKey(
_account->url().toString(),
keyWithIndex,
_keychainMigration ? QString() : _account->id()
) : keyWithIndex;

auto job = new QKeychain::DeletePasswordJob(_serviceName, this);
const auto job = new QKeychain::DeletePasswordJob(_serviceName, this);
#if defined(KEYCHAINCHUNK_ENABLE_INSECURE_FALLBACK)
addSettingsToJob(_account, job);
#endif
Expand Down

0 comments on commit d7cd797

Please sign in to comment.