diff --git a/src/libsync/abstractnetworkjob.cpp b/src/libsync/abstractnetworkjob.cpp index f5695fec8..7365c97a3 100644 --- a/src/libsync/abstractnetworkjob.cpp +++ b/src/libsync/abstractnetworkjob.cpp @@ -29,6 +29,7 @@ #include #include +#include "common/asserts.h" #include "networkjobs.h" #include "account.h" #include "owncloudpropagator.h" @@ -193,6 +194,10 @@ void AbstractNetworkJob::slotFinished() #endif if (_reply->error() != QNetworkReply::NoError) { + + if (_account->credentials()->retryIfNeeded(this)) + return; + if (!_ignoreCredentialFailure || _reply->error() != QNetworkReply::AuthenticationRequiredError) { qCWarning(lcNetworkJob) << _reply->error() << errorString() << _reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); @@ -440,4 +445,20 @@ QString networkReplyErrorString(const QNetworkReply &reply) return AbstractNetworkJob::tr(R"(Server replied "%1 %2" to "%3 %4")").arg(QString::number(httpStatus), httpReason, requestVerb(reply), reply.request().url().toDisplayString()); } +void AbstractNetworkJob::retry() +{ + ENFORCE(_reply); + auto req = _reply->request(); + QUrl requestedUrl = req.url(); + QByteArray verb = requestVerb(*_reply); + qCInfo(lcNetworkJob) << "Restarting" << verb << requestedUrl; + resetTimeout(); + if (_requestBody) { + _requestBody->seek(0); + } + // The cookie will be added automatically, we don't want AccessManager::createRequest to duplicate them + req.setRawHeader("cookie", QByteArray()); + sendRequest(verb, requestedUrl, req, _requestBody); +} + } // namespace OCC diff --git a/src/libsync/abstractnetworkjob.h b/src/libsync/abstractnetworkjob.h index c169e5451..b414b55bc 100644 --- a/src/libsync/abstractnetworkjob.h +++ b/src/libsync/abstractnetworkjob.h @@ -91,6 +91,9 @@ public: */ QString errorStringParsingBody(QByteArray *body = nullptr); + /** Make a new request */ + void retry(); + /** static variable the HTTP timeout (in seconds). If set to 0, the default will be used */ static int httpTimeout; diff --git a/src/libsync/creds/abstractcredentials.h b/src/libsync/creds/abstractcredentials.h index 896626c12..d679ac6be 100644 --- a/src/libsync/creds/abstractcredentials.h +++ b/src/libsync/creds/abstractcredentials.h @@ -25,6 +25,8 @@ class QNetworkAccessManager; class QNetworkReply; namespace OCC { +class AbstractNetworkJob; + class OWNCLOUDSYNC_EXPORT AbstractCredentials : public QObject { Q_OBJECT @@ -87,6 +89,9 @@ public: static QString keychainKey(const QString &url, const QString &user, const QString &accountId); + /** If the job need to be restarted or queue, this does it and returns true. */ + virtual bool retryIfNeeded(AbstractNetworkJob *) { return false; } + Q_SIGNALS: /** Emitted when fetchFromKeychain() is done. * diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp index 551d2f3cd..4fb16792d 100644 --- a/src/libsync/creds/httpcredentials.cpp +++ b/src/libsync/creds/httpcredentials.cpp @@ -45,6 +45,7 @@ namespace { const char clientCertificatePEMC[] = "_clientCertificatePEM"; const char clientKeyPEMC[] = "_clientKeyPEM"; const char authenticationFailedC[] = "owncloud-authentication-failed"; + const char needRetryC[] = "owncloud-need-retry"; } // ns class HttpCredentialsAccessManager : public AccessManager @@ -84,8 +85,15 @@ protected: req.setSslConfiguration(sslConfiguration); } + auto *reply = AccessManager::createRequest(op, req, outgoingData); - return AccessManager::createRequest(op, req, outgoingData); + if (_cred->_isRenewingOAuthToken) { + // We know this is going to fail, but we have no way to queue it there, so we will + // simply restart the job after the failure. + reply->setProperty(needRetryC, true); + } + + return reply; } private: @@ -362,6 +370,7 @@ bool HttpCredentials::refreshAccessToken() QString basicAuth = QString("%1:%2").arg( Theme::instance()->oauthClientId(), Theme::instance()->oauthClientSecret()); req.setRawHeader("Authorization", "Basic " + basicAuth.toUtf8().toBase64()); + req.setAttribute(HttpCredentials::DontAddCredentialsAttribute, true); auto requestBody = new QBuffer; QUrlQuery arguments(QString("grant_type=refresh_token&refresh_token=%1").arg(_refreshToken)); @@ -388,8 +397,15 @@ bool HttpCredentials::refreshAccessToken() _refreshToken = json["refresh_token"].toString(); persist(); } + _isRenewingOAuthToken = false; + for (const auto &job : _retryQueue) { + if (job) + job->retry(); + } + _retryQueue.clear(); emit fetched(); }); + _isRenewingOAuthToken = true; return true; } @@ -517,7 +533,27 @@ void HttpCredentials::slotAuthentication(QNetworkReply *reply, QAuthenticator *a // Thus, if we reach this signal, those credentials were invalid and we terminate. qCWarning(lcHttpCredentials) << "Stop request: Authentication failed for " << reply->url().toString(); reply->setProperty(authenticationFailedC, true); - reply->close(); + + if (_isRenewingOAuthToken) { + reply->setProperty(needRetryC, true); + } else if (isUsingOAuth() && !reply->property(needRetryC).toBool()) { + reply->setProperty(needRetryC, true); + qCInfo(lcHttpCredentials) << "Refreshing token"; + refreshAccessToken(); + } +} + +bool HttpCredentials::retryIfNeeded(AbstractNetworkJob *job) +{ + auto *reply = job->reply(); + if (!reply || !reply->property(needRetryC).toBool()) + return false; + if (_isRenewingOAuthToken) { + _retryQueue.append(job); + } else { + job->retry(); + } + return true; } } // namespace OCC diff --git a/src/libsync/creds/httpcredentials.h b/src/libsync/creds/httpcredentials.h index 45c350fa2..be74f9e51 100644 --- a/src/libsync/creds/httpcredentials.h +++ b/src/libsync/creds/httpcredentials.h @@ -107,6 +107,8 @@ public: // Whether we are using OAuth bool isUsingOAuth() const { return !_refreshToken.isNull(); } + bool retryIfNeeded(AbstractNetworkJob *) override; + private Q_SLOTS: void slotAuthentication(QNetworkReply *, QAuthenticator *); @@ -138,10 +140,13 @@ protected: QString _fetchErrorString; bool _ready = false; + bool _isRenewingOAuthToken = false; QSslKey _clientSslKey; QSslCertificate _clientSslCertificate; bool _keychainMigration = false; bool _retryOnKeyChainError = true; // true if we haven't done yet any reading from keychain + + QVector> _retryQueue; // Jobs we need to retry once the auth token is fetched }; diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index b276a8928..05a4c89bf 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -157,9 +157,16 @@ void GETFileJob::slotMetaDataChanged() int httpStatus = reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); - // Ignore redirects - if (httpStatus == 301 || httpStatus == 302 || httpStatus == 303 || httpStatus == 307 || httpStatus == 308) + if (httpStatus == 301 || httpStatus == 302 || httpStatus == 303 || httpStatus == 307 + || httpStatus == 308 || httpStatus == 401) { + // Redirects and auth failures (oauth token renew) are handled by AbstractNetworkJob and + // will end up restarting the job. We do not want to process further data from the initial + // request. newReplyHook() will reestablish signal connections for the follow-up request. + bool ok = disconnect(reply(), &QNetworkReply::finished, this, &GETFileJob::slotReadyRead) + && disconnect(reply(), &QNetworkReply::readyRead, this, &GETFileJob::slotReadyRead); + ASSERT(ok); return; + } // If the status code isn't 2xx, don't write the reply body to the file. // For any error: handle it when the job is finished, not here. diff --git a/src/libsync/propagatedownload.h b/src/libsync/propagatedownload.h index 86635d087..856656ddd 100644 --- a/src/libsync/propagatedownload.h +++ b/src/libsync/propagatedownload.h @@ -67,7 +67,7 @@ public: void start() override; bool finished() override { - if (reply()->bytesAvailable()) { + if (_saveBodyToFile && reply()->bytesAvailable()) { return false; } else { if (_bandwidthManager) {