CheckServerJob: clear cookies under more circumstances

8 files changed, 41 insertions, 11 deletions

diff --git a/changelog/unreleased/9489 b/changelog/unreleased/9489
new file mode 100644
index 000000000..06075e1be
--- /dev/null
+++ b/changelog/unreleased/9489
@@ -0,0 +1,7 @@
+Bugfix: If reuqired clear cookies in more scenarios
+
+BigIp F5 requires special cookie handling on our side.
+We only explicitly cleared the cookies when we hit an unexpected redirect,
+now we will clear them also when refreshing our OAuth token.
+
+https://github.com/owncloud/client/pull/9489
diff --git a/src/gui/accountstate.cpp b/src/gui/accountstate.cpp
index 9a42c0cc2..b04181a97 100644
--- a/src/gui/accountstate.cpp
+++ b/src/gui/accountstate.cpp
@@ -311,10 +311,7 @@ void AccountState::checkConnectivity(bool blockJobs)
         // Use a small authed propfind as a minimal ping when we're
         // already connected.
         if (blockJobs) {
-            if (Theme::instance()->connectionValidatorClearCookies()) {
-                // clear the cookies directly before we try to validate
-                connect(_connectionValidator, &ConnectionValidator::aboutToStart, _account.get(), &Account::clearCookieJar, Qt::DirectConnection);
-            }
+            _connectionValidator->setClearCookies(true);
             _connectionValidator->checkServer();
         } else {
             _connectionValidator->checkServerAndUpdate();
@@ -335,6 +332,7 @@ void AccountState::checkConnectivity(bool blockJobs)
         // ssl config that does not have a sensible certificate chain.
         account()->setSslConfiguration(QSslConfiguration());
         //#endif
+        account()->clearCookieJar();
         _connectionValidator->checkServerAndUpdate();
     }
 }
diff --git a/src/gui/connectionvalidator.cpp b/src/gui/connectionvalidator.cpp
index 88479cf09..876c4d059 100644
--- a/src/gui/connectionvalidator.cpp
+++ b/src/gui/connectionvalidator.cpp
@@ -39,6 +39,11 @@ ConnectionValidator::ConnectionValidator(AccountPtr account, QObject *parent)
 {
 }
 
+void ConnectionValidator::setClearCookies(bool clearCookies)
+{
+    _clearCookies = clearCookies;
+}
+
 void ConnectionValidator::checkServer()
 {
     _updateConfig = false;
@@ -88,6 +93,7 @@ void ConnectionValidator::systemProxyLookupDone(const QNetworkProxy &proxy)
 void ConnectionValidator::slotCheckServerAndAuth()
 {
     CheckServerJob *checkJob = new CheckServerJob(_account, this);
+    checkJob->setClearCookies(_clearCookies);
     checkJob->setTimeout(timeoutToUseMsec);
     connect(checkJob, &CheckServerJob::instanceFound, this, &ConnectionValidator::slotStatusFound);
     connect(checkJob, &CheckServerJob::instanceNotFound, this, &ConnectionValidator::slotNoStatusFound);
@@ -96,7 +102,6 @@ void ConnectionValidator::slotCheckServerAndAuth()
         _errors.append(tr("timeout"));
         reportResult(Timeout);
     });
-    Q_EMIT aboutToStart();
     checkJob->start();
 }
 
diff --git a/src/gui/connectionvalidator.h b/src/gui/connectionvalidator.h
index 41847245e..e06a4f97b 100644
--- a/src/gui/connectionvalidator.h
+++ b/src/gui/connectionvalidator.h
@@ -99,6 +99,12 @@ public:
     // How often should the Application ask this object to check for the connection?
     enum { DefaultCallingIntervalMsec = 62 * 1000 };
 
+
+    /** Whether to clear the cookies before we start the CheckServerJob job
+     * This option also depends on Theme::instance()->connectionValidatorClearCookies()
+     */
+    void setClearCookies(bool clearCookies);
+
 public slots:
     /// Checks the server and the authentication.
     void checkServer();
@@ -106,10 +112,6 @@ public slots:
     void systemProxyLookupDone(const QNetworkProxy &proxy);
 
 signals:
-    /**
-     * Emited before the actual validation starts
-     */
-    void aboutToStart();
     void connectionResult(ConnectionValidator::Status status, const QStringList &errors);
 
 protected slots:
@@ -143,6 +145,7 @@ private:
     QStringList _errors;
     AccountPtr _account;
     bool _updateConfig = true;
+    bool _clearCookies = false;
 };
 }
 
diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp
index 6d014beeb..06530a228 100644
--- a/src/libsync/account.cpp
+++ b/src/libsync/account.cpp
@@ -209,6 +209,7 @@ void Account::clearCookieJar()
 {
     auto jar = qobject_cast<CookieJar *>(_am->cookieJar());
     OC_ASSERT(jar);
+    qCInfo(lcAccount) << "Clearing cookies";
     jar->setAllCookies(QList<QNetworkCookie>());
     emit wantsAccountSaved(this);
 }
diff --git a/src/libsync/creds/oauth.cpp b/src/libsync/creds/oauth.cpp
index f911ca7c8..28ae21744 100644
--- a/src/libsync/creds/oauth.cpp
+++ b/src/libsync/creds/oauth.cpp
@@ -472,6 +472,7 @@ void OAuth::authorisationLinkAsync(std::function<void (const QUrl &)> callback)
 void OAuth::fetchWellKnown()
 {
     auto checkServer = new CheckServerJob(_account->sharedFromThis(), this);
+    checkServer->setClearCookies(true);
     checkServer->setTimeout(qMin(30 * 1000ll, checkServer->timeoutMsec()));
     connect(checkServer, &CheckServerJob::instanceNotFound, this, [this](QNetworkReply *reply) {
         if (_isRefreshingToken) {
diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp
index 2a16be727..41de2d467 100644
--- a/src/libsync/networkjobs.cpp
+++ b/src/libsync/networkjobs.cpp
@@ -40,6 +40,8 @@
 
 #include "creds/abstractcredentials.h"
 #include "creds/httpcredentials.h"
+#include "theme.h"
+
 
 namespace OCC {
 
@@ -415,7 +417,6 @@ const QHash<QString, qint64> &LsColJob::sizes() const
 
 CheckServerJob::CheckServerJob(AccountPtr account, QObject *parent)
     : AbstractNetworkJob(account, QStringLiteral("status.php"), parent)
-    , _subdirFallback(false)
 {
     setIgnoreCredentialFailure(true);
     setAuthenticationJob(true);
@@ -430,10 +431,19 @@ void CheckServerJob::start()
     req.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy);
     req.setRawHeader(QByteArrayLiteral("OC-Connection-Validator"), QByteArrayLiteral("desktop"));
     req.setMaximumRedirectsAllowed(_maxRedirectsAllowed);
+
+    if (_clearCookies && Theme::instance()->connectionValidatorClearCookies()) {
+        _account->clearCookieJar();
+    }
     sendRequest("GET", Utility::concatUrlPath(_serverUrl, path()), req);
     AbstractNetworkJob::start();
 }
 
+void CheckServerJob::setClearCookies(bool clearCookies)
+{
+    _clearCookies = clearCookies;
+}
+
 void CheckServerJob::onTimedOut()
 {
     qCWarning(lcCheckServerJob) << "TIMEOUT";
diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h
index 7dab57309..69c9898fc 100644
--- a/src/libsync/networkjobs.h
+++ b/src/libsync/networkjobs.h
@@ -256,6 +256,11 @@ public:
     int maxRedirectsAllowed() const;
     void setMaxRedirectsAllowed(int maxRedirectsAllowed);
 
+    /** Whether to clear the cookies before we start the job
+     * This option also depends on Theme::instance()->connectionValidatorClearCookies()
+     */
+    void setClearCookies(bool clearCookies);
+
 signals:
     /** Emitted when a status.php was successfully read.
      *
@@ -287,7 +292,7 @@ protected:
     void newReplyHook(QNetworkReply *) override;
 
 private:
-    bool _subdirFallback;
+    bool _clearCookies = false;
 
     /** The permanent-redirect adjusted account url.
      *