diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp index c9a2c2f345..d8d088142a 100644 --- a/src/libsync/creds/httpcredentials.cpp +++ b/src/libsync/creds/httpcredentials.cpp @@ -233,14 +233,17 @@ bool HttpCredentials::refreshAccessTokenInternal(int tokenRefreshRetriesCount) if (nextTry >= TokenRefreshMaxRetries) { qCWarning(lcHttpCredentials) << u"Too many failed refreshes" << nextTry << u"-> log out"; forgetSensitiveData(); + // terminal failure: clears the job queue (see Account) and logs out Q_EMIT authenticationFailed(); Q_EMIT fetched(); - return; + } else { + // Transient failure: retry. Do NOT emit authenticationFailed() here -- it + // would clear the job queue and abort in-flight uploads, silently dropping + // files when the run then finalizes as complete (opencloud-eu/desktop#900, #948). + QTimer::singleShot(timeout, this, [nextTry, this] { + refreshAccessTokenInternal(nextTry); + }); } - QTimer::singleShot(timeout, this, [nextTry, this] { - refreshAccessTokenInternal(nextTry); - }); - Q_EMIT authenticationFailed(); }); connect(_oAuthJob, &AccountBasedOAuth::refreshFinished, this, [this](const QString &accessToken, const QString &refreshToken) { diff --git a/test/testoauth/testoauth.cpp b/test/testoauth/testoauth.cpp index 6df7ae0998..a7068d3050 100644 --- a/test/testoauth/testoauth.cpp +++ b/test/testoauth/testoauth.cpp @@ -9,6 +9,7 @@ #include #include "common/asserts.h" +#include "libsync/creds/httpcredentials.h" #include "libsync/creds/oauth.h" #include "testutils/syncenginetestutils.h" #include "theme.h" @@ -308,6 +309,32 @@ class OAuthTestCase : public QObject } }; +class TestHttpCredentials : public HttpCredentials +{ +public: + TestHttpCredentials(FakeAM::Override override, const QString &refreshToken) + : HttpCredentials(QStringLiteral("initial-access-token")) + , _override(std::move(override)) + { + _refreshToken = refreshToken; + } + + OCC::AccessManager *createAM() const override + { + auto *am = new FakeAM({}, nullptr); + am->setOverride(_override); + return am; + } + + void restartOauth() override { } + void fetchFromKeychain() override { } + void persist() override { } + +private: + FakeAM::Override _override; +}; + + class TestOAuth : public QObject { Q_OBJECT @@ -757,6 +784,103 @@ private Q_SLOTS: } test; test.test(); } + + void testTransientRefreshDoesNotEmitAuthFailed() + { + QObject replyParent; + int tokenRequestCount = 0; + + // Use TimeoutError: resets nextTry to 0, so it never reaches TokenRefreshMaxRetries, + // and has TokenRefreshDefaultTimeout (30s) so no retry fires during our test window. + auto override = [&replyParent, &tokenRequestCount](QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *) -> QNetworkReply * { + if (req.url().path().endsWith(QLatin1String("status.php"))) { + const QJsonDocument json(QJsonObject{ + {QStringLiteral("installed"), true}, + {QStringLiteral("maintenance"), false}, + {QStringLiteral("version"), QStringLiteral("10.0.0")}, + }); + return new FakePayloadReply(op, req, json.toJson(), &replyParent); + } + if (req.url().path().endsWith(QLatin1String(".well-known/openid-configuration"))) { + const QJsonDocument json(QJsonObject{ + {QStringLiteral("authorization_endpoint"), QStringLiteral("oauthtest://openidserver/authorize")}, + {QStringLiteral("token_endpoint"), QStringLiteral("oauthtest://openidserver/token_endpoint")}, + }); + return new FakePayloadReply(op, req, json.toJson(), &replyParent); + } + if (req.url().toString().contains(QLatin1String("token_endpoint"))) { + ++tokenRequestCount; + auto *reply = new FakeErrorReply(op, req, &replyParent, 408); + reply->setError(QNetworkReply::TimeoutError, QStringLiteral("Request timed out")); + return reply; + } + return new FakePayloadReply(op, req, {}, &replyParent); + }; + + auto account = Account::create(QUuid::createUuid()); + account->setUrl(sOAuthTestServer); + auto *creds = new TestHttpCredentials(override, QStringLiteral("fake-refresh-token")); + account->setCredentials(creds); + + QSignalSpy authFailedSpy(creds, &AbstractCredentials::authenticationFailed); + QSignalSpy authStartedSpy(creds, &AbstractCredentials::authenticationStarted); + + QVERIFY(creds->refreshAccessToken()); + + // Wait for the first token request to complete (queued connections fire). + // TimeoutError → nextTry resets to 0, retry scheduled in 30s — no retry in this window. + QTRY_VERIFY_WITH_TIMEOUT(tokenRequestCount >= 1, 5000); + QCOMPARE(authStartedSpy.count(), 1); + // The fix: transient errors must NOT emit authenticationFailed + QCOMPARE(authFailedSpy.count(), 0); + } + + void testTerminalRefreshEmitsAuthFailed() + { + QObject replyParent; + int tokenRequestCount = 0; + + auto override = [&replyParent, &tokenRequestCount](QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *) -> QNetworkReply * { + if (req.url().path().endsWith(QLatin1String("status.php"))) { + const QJsonDocument json(QJsonObject{ + {QStringLiteral("installed"), true}, + {QStringLiteral("maintenance"), false}, + {QStringLiteral("version"), QStringLiteral("10.0.0")}, + }); + return new FakePayloadReply(op, req, json.toJson(), &replyParent); + } + if (req.url().path().endsWith(QLatin1String(".well-known/openid-configuration"))) { + const QJsonDocument json(QJsonObject{ + {QStringLiteral("authorization_endpoint"), QStringLiteral("oauthtest://openidserver/authorize")}, + {QStringLiteral("token_endpoint"), QStringLiteral("oauthtest://openidserver/token_endpoint")}, + }); + return new FakePayloadReply(op, req, json.toJson(), &replyParent); + } + if (req.url().toString().contains(QLatin1String("token_endpoint"))) { + ++tokenRequestCount; + auto *reply = new FakeErrorReply(op, req, &replyParent, 404); + reply->setError(QNetworkReply::ContentNotFoundError, QStringLiteral("Not found")); + return reply; + } + return new FakePayloadReply(op, req, {}, &replyParent); + }; + + auto account = Account::create(QUuid::createUuid()); + account->setUrl(sOAuthTestServer); + auto *creds = new TestHttpCredentials(override, QStringLiteral("fake-refresh-token")); + account->setCredentials(creds); + + QSignalSpy authFailedSpy(creds, &AbstractCredentials::authenticationFailed); + QSignalSpy fetchedSpy(creds, &AbstractCredentials::fetched); + + QVERIFY(creds->refreshAccessToken()); + + // ContentNotFoundError → timeout=0s, nextTry increments each time. + // After TokenRefreshMaxRetries (3) errors, terminal branch emits authenticationFailed. + QTRY_VERIFY_WITH_TIMEOUT(authFailedSpy.count() == 1, 10000); + QCOMPARE(fetchedSpy.count(), 1); + QVERIFY(tokenRequestCount >= 3); + } };