From ccae9c172446c6403426c629d133bdeed1e2946c Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Fri, 21 Jan 2022 14:30:46 +0100 Subject: [PATCH 1/2] Ignore network/http errors and retry to refresh the token multiple times Fixes: #9245 --- changelog/unreleased/9245 | 4 +++ src/libsync/creds/httpcredentials.cpp | 43 +++++++++++---------------- src/libsync/creds/httpcredentials.h | 1 + 3 files changed, 22 insertions(+), 26 deletions(-) create mode 100644 changelog/unreleased/9245 diff --git a/changelog/unreleased/9245 b/changelog/unreleased/9245 new file mode 100644 index 00000000000..fbc66d6cee6 --- /dev/null +++ b/changelog/unreleased/9245 @@ -0,0 +1,4 @@ +Enhancement: Retry token refresh multiple times before logout + +https://github.com/owncloud/client/issues/9245 +https://github.com/owncloud/client/pull/9380 diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp index f463f8e9f24..33f634e753a 100644 --- a/src/libsync/creds/httpcredentials.cpp +++ b/src/libsync/creds/httpcredentials.cpp @@ -34,10 +34,15 @@ #include #include +#include + +using namespace std::chrono_literals; + Q_LOGGING_CATEGORY(lcHttpCredentials, "sync.credentials.http", QtInfoMsg) Q_LOGGING_CATEGORY(lcHttpLegacyCredentials, "sync.credentials.http.legacy", QtInfoMsg) namespace { +constexpr int TokenRefreshMaxRetries = 3; constexpr int CredentialVersion = 1; const char authenticationFailedC[] = "owncloud-authentication-failed"; @@ -241,6 +246,13 @@ bool HttpCredentials::refreshAccessToken() if (_isRenewingOAuthToken) { return true; } + if (++_tokenRefreshRetriesCount >= TokenRefreshMaxRetries) { + qCWarning(lcHttpCredentials) << "Too many failed refreshs" << _tokenRefreshRetriesCount << "-> log out"; + forgetSensitiveData(); + Q_EMIT _account->invalidCredentials(); + _tokenRefreshRetriesCount = 0; + return false; + } _isRenewingOAuthToken = true; // don't touch _ready or the account state will start a new authentication @@ -251,34 +263,12 @@ bool HttpCredentials::refreshAccessToken() oauth->deleteLater(); _isRenewingOAuthToken = false; switch (error) { - // most probably a timeout - case QNetworkReply::OperationCanceledError: - Q_FALLTHROUGH(); - case QNetworkReply::RemoteHostClosedError: - Q_FALLTHROUGH(); - case QNetworkReply::ConnectionRefusedError: - Q_FALLTHROUGH(); - case QNetworkReply::HostNotFoundError: - Q_FALLTHROUGH(); - case QNetworkReply::TimeoutError: - Q_FALLTHROUGH(); - case QNetworkReply::TemporaryNetworkFailureError: - Q_FALLTHROUGH(); - case QNetworkReply::NetworkSessionFailedError: - Q_FALLTHROUGH(); - case QNetworkReply::InternalServerError: - Q_FALLTHROUGH(); - case QNetworkReply::ServiceUnavailableError: - Q_FALLTHROUGH(); - case QNetworkReply::UnknownNetworkError: - QTimer::singleShot(30000, this, &HttpCredentials::refreshAccessToken); + case QNetworkReply::ContentNotFoundError: + // 404: bigip f5? + QTimer::singleShot(0, this, &HttpCredentials::refreshAccessToken); break; default: - // something is broken - // start fresh - qCWarning(lcHttpCredentials) << "Token refresh encountered an unsupported network error" << errorString << "-> log out"; - forgetSensitiveData(); - Q_EMIT _account->invalidCredentials(); + QTimer::singleShot(30s, this, &HttpCredentials::refreshAccessToken); } Q_EMIT authenticationFailed(); }); @@ -286,6 +276,7 @@ bool HttpCredentials::refreshAccessToken() connect(oauth, &OAuth::refreshFinished, this, [this, oauth](const QString &accessToken, const QString &refreshToken){ oauth->deleteLater(); _isRenewingOAuthToken = false; + _tokenRefreshRetriesCount = 0; if (refreshToken.isEmpty()) { // an error occured, log out forgetSensitiveData(); diff --git a/src/libsync/creds/httpcredentials.h b/src/libsync/creds/httpcredentials.h index 8b9f7260dd7..5ca4d5c4f34 100644 --- a/src/libsync/creds/httpcredentials.h +++ b/src/libsync/creds/httpcredentials.h @@ -102,6 +102,7 @@ class OWNCLOUDSYNC_EXPORT HttpCredentials : public AbstractCredentials DetermineAuthTypeJob::AuthType _authType = DetermineAuthTypeJob::AuthType::Unknown; private: + int _tokenRefreshRetriesCount = 0; // HttpLegacyCredentials is incompelte QPointer _credentialMigration; }; From 80873df28ec10e317c7cb2537569e4326bc4de2f Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Fri, 21 Jan 2022 16:38:47 +0100 Subject: [PATCH 2/2] Don't translate log message --- src/libsync/creds/oauth.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libsync/creds/oauth.cpp b/src/libsync/creds/oauth.cpp index e37df23cb1f..f911ca7c8c6 100644 --- a/src/libsync/creds/oauth.cpp +++ b/src/libsync/creds/oauth.cpp @@ -331,21 +331,21 @@ void OAuth::refreshAuthentication(const QString &refreshToken) error == QLatin1String("invalid_request")) { newRefreshToken.clear(); } else { - qCWarning(lcOauth) << tr("Error while refreshing the token: %1 : %2").arg(error, data.value(QStringLiteral("error_description")).toString()); + qCWarning(lcOauth) << "Error while refreshing the token:" << error << data.value(QStringLiteral("error_description")).toString(); } } else if (reply->error() != QNetworkReply::NoError) { - qCWarning(lcOauth) << tr("Error while refreshing the token: %1 : %2").arg(reply->errorString(), QString::fromUtf8(jsonData)); + qCWarning(lcOauth) << "Error while refreshing the token:" << reply->error() << ":" << reply->errorString() << reply->attribute(QNetworkRequest::HttpStatusCodeAttribute) << jsonData; Q_EMIT refreshError(reply->error(), reply->errorString()); return; } else { if (jsonParseError.error != QJsonParseError::NoError || data.isEmpty()) { // Invalid or empty JSON: Network error maybe? - qCWarning(lcOauth) << tr("Error while refreshing the token: %1 : %2").arg(jsonParseError.errorString(), QString::fromUtf8(jsonData)); + qCWarning(lcOauth) << "Error while refreshing the token:" << jsonParseError.errorString() << jsonData; } else { QString error; accessToken = getRequiredField(data, QStringLiteral("access_token"), &error).toString(); if (!error.isEmpty()) { - qCWarning(lcOauth) << tr("The reply from the server did not contain all expected fields\n:%1\nReceived data: %2").arg(error, QString::fromUtf8(jsonData)); + qCWarning(lcOauth) << "The reply from the server did not contain all expected fields:" << error << "received data:" << jsonData; } const auto refresh_token = data.find(QStringLiteral("refresh_token"));