Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A 404 during a token refresh is no reason to give up #9380

Merged
merged 2 commits into from
Jan 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/unreleased/9245
Original file line number Diff line number Diff line change
@@ -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
43 changes: 17 additions & 26 deletions src/libsync/creds/httpcredentials.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,15 @@
#include <QSettings>
#include <QSslKey>

#include <chrono>

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";

Expand Down Expand Up @@ -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
Expand All @@ -251,41 +263,20 @@ 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();
});

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();
Expand Down
1 change: 1 addition & 0 deletions src/libsync/creds/httpcredentials.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ class OWNCLOUDSYNC_EXPORT HttpCredentials : public AbstractCredentials
DetermineAuthTypeJob::AuthType _authType = DetermineAuthTypeJob::AuthType::Unknown;

private:
int _tokenRefreshRetriesCount = 0;
// HttpLegacyCredentials is incompelte
QPointer<QObject> _credentialMigration;
};
Expand Down
8 changes: 4 additions & 4 deletions src/libsync/creds/oauth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down