Skip to content

Commit

Permalink
Fix remote wipe keychain storage (issue #1592)
Browse files Browse the repository at this point in the history
The app password for the remote wipe was constantly being written in
WebFlowCredentials::slotFinished to the keychain, leading to unnecessary
write and log overhead on the system.

This fix introduces a check to only store the app password once in
a lifetime of the Account class. Also the method used to store the
password will be renamed from setAppPassword to writeAppPasswordOnce
to be more expressive.

Signed-off-by: Michael Schuster <michael@schuster.ms>
  • Loading branch information
misch7 authored and Camila Ayres committed Nov 29, 2019
1 parent 8ae18d9 commit dcc84d3
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/gui/creds/webflowcredentials.cpp
Expand Up @@ -420,7 +420,7 @@ void WebFlowCredentials::slotFinished(QNetworkReply *reply) {
_credentialsValid = true;

/// Used later for remote wipe
_account->setAppPassword(_password);
_account->writeAppPasswordOnce(_password);
}
}

Expand Down
9 changes: 7 additions & 2 deletions src/libsync/account.cpp
Expand Up @@ -513,7 +513,10 @@ void Account::setNonShib(bool nonShib)
}
}

void Account::setAppPassword(QString appPassword){
void Account::writeAppPasswordOnce(QString appPassword){
if(_wroteAppPassword)
return;

const QString kck = AbstractCredentials::keychainKey(
url().toString(),
davUser() + app_password,
Expand All @@ -524,8 +527,10 @@ void Account::setAppPassword(QString appPassword){
job->setInsecureFallback(false);
job->setKey(kck);
job->setBinaryData(appPassword.toLatin1());
connect(job, &WritePasswordJob::finished, [](Job *) {
connect(job, &WritePasswordJob::finished, [this](Job *) {
qCInfo(lcAccount) << "appPassword stored in keychain";

_wroteAppPassword = true;
});
job->start();
}
Expand Down
5 changes: 4 additions & 1 deletion src/libsync/account.h
Expand Up @@ -243,7 +243,7 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject

/// Used in RemoteWipe
void retrieveAppPassword();
void setAppPassword(QString appPassword);
void writeAppPasswordOnce(QString appPassword);
void deleteAppPassword();

public slots:
Expand Down Expand Up @@ -319,6 +319,9 @@ protected Q_SLOTS:
QString _davPath; // defaults to value from theme, might be overwritten in brandings
ClientSideEncryption _e2e;

/// Used in RemoteWipe
bool _wroteAppPassword = false;

friend class AccountManager;
};
}
Expand Down

0 comments on commit dcc84d3

Please sign in to comment.