OAuth2: Refresh the token without aborting the sync

OAuth2 access token typically only has a token valid for 1 hour.
Before this patch, when the token was timing out during the sync, the
sync was aborted, and the ConnectionValidator was then requesting a new
token, so the sync can be started over.
If the discovery takes longer than the oauth2 validity, this means that
the sync can never proceed, as it would be always restarted from scratch.

With this patch, we try to transparently renew the OAuth2 token and restart
the jobs that failed because the access token was invalid.

Note that some changes were required in the GETFile job because it handled
the error itself and so it was erroring the jobs before its too late.

Issue #6814

Author: ogoffart

src/libsync/abstractnetworkjob.cpp

@@ -29,6 +29,7 @@
 #include <QAuthenticator>
 #include <QMetaEnum>
 
+#include "common/asserts.h"
 #include "networkjobs.h"
 #include "account.h"
 #include "owncloudpropagator.h"
@@ -161,6 +162,10 @@ void AbstractNetworkJob::slotFinished()
     }
 
     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);
@@ -408,4 +413,20 @@ QString networkReplyErrorString(const QNetworkReply &reply)
     return AbstractNetworkJob::tr("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

src/libsync/abstractnetworkjob.h

@@ -90,6 +90,9 @@ class OWNCLOUDSYNC_EXPORT AbstractNetworkJob : public QObject
      */
     QString errorStringParsingBody(QByteArray *body = 0);
 
+    /** 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;

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 @@ class OWNCLOUDSYNC_EXPORT AbstractCredentials : public QObject
 
     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.
      *

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 @@ class HttpCredentialsAccessManager : public AccessManager
             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:
@@ -361,6 +369,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));
@@ -387,8 +396,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;
 }
 
@@ -516,7 +532,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

src/libsync/creds/httpcredentials.h

@@ -107,6 +107,8 @@ class OWNCLOUDSYNC_EXPORT HttpCredentials : public AbstractCredentials
     // 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 @@ private Q_SLOTS:
 
     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<QPointer<AbstractNetworkJob>> _retryQueue; // Jobs we need to retry once the auth token is fetched
 };
 
 

src/libsync/propagatedownload.cpp

@@ -159,9 +159,14 @@ 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) {
+        // Ignore redirects and authentication failures.
+        // This is handled by AbstractNetworkJob,  so don't let this job error out in this case.
+        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.

src/libsync/propagatedownload.h

@@ -65,7 +65,7 @@ class GETFileJob : public AbstractNetworkJob
     virtual void start() Q_DECL_OVERRIDE;
     virtual bool finished() Q_DECL_OVERRIDE
     {
-        if (reply()->bytesAvailable()) {
+        if (_saveBodyToFile && reply()->bytesAvailable()) {
             return false;
         } else {
             if (_bandwidthManager) {