From efef49ee1fa2cfc1b08b784b9737789110dcab74 Mon Sep 17 00:00:00 2001 From: Erik Verbruggen Date: Thu, 11 Apr 2024 17:39:35 +0200 Subject: [PATCH] Stop synchronisation when behind a captive portal Fixes: #11533 --- src/gui/accountsettings.cpp | 6 ++++- src/gui/accountstate.cpp | 30 +++++++++++++++++++++ src/gui/connectionvalidator.cpp | 5 ++-- src/gui/connectionvalidator.h | 5 ++-- src/gui/networkinformation.cpp | 48 ++++++++++++++++++++++++++++++++- src/gui/networkinformation.h | 16 +++++++++++ src/gui/owncloudgui.cpp | 6 +++++ test/testutils/testutils.cpp | 3 +++ 8 files changed, 113 insertions(+), 6 deletions(-) diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index 4e70a5286f4..bb09e7eca34 100644 --- a/src/gui/accountsettings.cpp +++ b/src/gui/accountsettings.cpp @@ -491,7 +491,11 @@ void AccountSettings::slotAccountStateChanged() break; } case AccountState::Connecting: - showConnectionLabel(tr("Connecting to: %1.").arg(server)); + if (NetworkInformation::instance()->isBehindCaptivePortal()) { + showConnectionLabel(tr("Captive portal prevents connections to %1.").arg(server)); + } else { + showConnectionLabel(tr("Connecting to: %1.").arg(server)); + } break; case AccountState::ConfigurationError: showConnectionLabel(tr("Server configuration error: %1.") diff --git a/src/gui/accountstate.cpp b/src/gui/accountstate.cpp index 17b0c795677..acbd6d67e60 100644 --- a/src/gui/accountstate.cpp +++ b/src/gui/accountstate.cpp @@ -146,6 +146,12 @@ AccountState::AccountState(AccountPtr account) } }); + connect(NetworkInformation::instance(), &NetworkInformation::isBehindCaptivePortalChanged, this, [this](bool) { + // A direct connect is not possible, because then the state parameter of `isBehindCaptivePortalChanged` + // would become the `verifyServerState` argument to `checkConnectivity`. + QTimer::singleShot(0, this, [this] { checkConnectivity(false); }); + }); + // as a fallback and to recover after server issues we also poll auto timer = new QTimer(this); timer->setInterval(ConnectionValidator::DefaultCallingInterval); @@ -166,6 +172,22 @@ AccountState::AccountState(AccountPtr account) ownCloudGui::raise(); msgBox->open(); }); + + + connect(NetworkInformation::instance(), &NetworkInformation::isBehindCaptivePortalChanged, this, [this](bool onoff) { + if (onoff) { + _queueGuard.block(); + } else { + // TODO: empty queue? + _queueGuard.unblock(); + } + }); + if (NetworkInformation::instance()->isBehindCaptivePortal()) { + _queueGuard.block(); + } else { + // TODO: empty queue? + _queueGuard.unblock(); + } } AccountState::~AccountState() { } @@ -362,6 +384,9 @@ void AccountState::checkConnectivity(bool blockJobs) this, &AccountState::slotConnectionValidatorResult); connect(_connectionValidator, &ConnectionValidator::sslErrors, this, [blockJobs, this](const QList &errors) { + if (NetworkInformation::instance()->isBehindCaptivePortal()) { + return; + } if (!_tlsDialog) { // ignore errors for already accepted certificates auto filteredErrors = _account->accessManager()->filterSslErrors(errors); @@ -477,6 +502,7 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta setState(Connected); break; case ConnectionValidator::Undefined: + [[fallthrough]]; case ConnectionValidator::NotConfigured: setState(Disconnected); break; @@ -492,6 +518,7 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta setState(NetworkError); break; case ConnectionValidator::CredentialsWrong: + [[fallthrough]]; case ConnectionValidator::CredentialsNotReady: slotInvalidCredentials(); break; @@ -509,6 +536,9 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta case ConnectionValidator::Timeout: setState(NetworkError); break; + case ConnectionValidator::CaptivePortal: + setState(Connecting); + break; } } diff --git a/src/gui/connectionvalidator.cpp b/src/gui/connectionvalidator.cpp index 899d054433b..09b7fe004f1 100644 --- a/src/gui/connectionvalidator.cpp +++ b/src/gui/connectionvalidator.cpp @@ -14,6 +14,7 @@ #include "gui/connectionvalidator.h" #include "gui/clientproxy.h" #include "gui/fetchserversettings.h" +#include "gui/networkinformation.h" #include "gui/tlserrordialog.h" #include "libsync/account.h" #include "libsync/cookiejar.h" @@ -132,7 +133,7 @@ void ConnectionValidator::slotCheckServerAndAuth() reportResult(Timeout); return; case QNetworkReply::SslHandshakeFailedError: - reportResult(SslError); + reportResult(NetworkInformation::instance()->isBehindCaptivePortal() ? CaptivePortal : SslError); return; case QNetworkReply::TooManyRedirectsError: reportResult(MaintenanceMode); @@ -214,7 +215,7 @@ void ConnectionValidator::slotAuthFailed(QNetworkReply *reply) if (reply->error() == QNetworkReply::SslHandshakeFailedError) { _errors << job->errorStringParsingBody(); - stat = SslError; + stat = NetworkInformation::instance()->isBehindCaptivePortal() ? CaptivePortal : SslError; } else if (reply->error() == QNetworkReply::AuthenticationRequiredError || !_account->credentials()->stillValid(reply)) { diff --git a/src/gui/connectionvalidator.h b/src/gui/connectionvalidator.h index f655d094f26..f04c12ac1e6 100644 --- a/src/gui/connectionvalidator.h +++ b/src/gui/connectionvalidator.h @@ -101,9 +101,10 @@ class OWNCLOUDGUI_EXPORT ConnectionValidator : public QObject ServiceUnavailable, // 503 on authed request MaintenanceMode, // maintenance enabled in status.php Timeout, // actually also used for other errors on the authed request - ClientUnsupported // The server blocks us as an unsupported client + ClientUnsupported, // The server blocks us as an unsupported client + CaptivePortal, // We're stuck behind a captive portal and (will) get SSL certificate problems }; - Q_ENUM(Status); + Q_ENUM(Status) // How often should the Application ask this object to check for the connection? static constexpr auto DefaultCallingInterval = std::chrono::seconds(62); diff --git a/src/gui/networkinformation.cpp b/src/gui/networkinformation.cpp index d39766bc567..af548338069 100644 --- a/src/gui/networkinformation.cpp +++ b/src/gui/networkinformation.cpp @@ -46,7 +46,9 @@ static void loadQNetworkInformationBackend() void NetworkInformation::initialize() { - Q_ASSERT(!_instance); + if (_instance) { + return; + } _instance = new NetworkInformation; @@ -55,6 +57,7 @@ void NetworkInformation::initialize() if (auto qni = QNetworkInformation::instance()) { connect(qni, &QNetworkInformation::isMeteredChanged, _instance, &NetworkInformation::isMeteredChanged); connect(qni, &QNetworkInformation::reachabilityChanged, _instance, &NetworkInformation::reachabilityChanged); + connect(qni, &QNetworkInformation::isBehindCaptivePortalChanged, _instance, &NetworkInformation::slotIsBehindCaptivePortalChanged); } } @@ -80,3 +83,46 @@ bool NetworkInformation::supports(Features features) const return false; } + +bool NetworkInformation::isForcedCaptivePortal() const +{ + return _forcedCaptivePortal; +} + +void NetworkInformation::setForcedCaptivePortal(bool onoff) +{ + if (_forcedCaptivePortal != onoff) { + _forcedCaptivePortal = onoff; + qCDebug(lcNetInfo) << "Switching forced captive portal to" << onoff; + + bool behindCaptivePortal = false; + + if (auto *qNetInfo = QNetworkInformation::instance()) { + behindCaptivePortal = qNetInfo->isBehindCaptivePortal(); + } + + if (behindCaptivePortal != _forcedCaptivePortal) { + Q_EMIT isBehindCaptivePortalChanged(_forcedCaptivePortal); + } + } +} + +bool NetworkInformation::isBehindCaptivePortal() const +{ + if (_forcedCaptivePortal) { + return true; + } + + if (auto *qNetInfo = QNetworkInformation::instance()) { + return qNetInfo->isBehindCaptivePortal(); + } + + return false; +} + +void NetworkInformation::slotIsBehindCaptivePortalChanged(bool state) +{ + if (!_forcedCaptivePortal) { + Q_EMIT isBehindCaptivePortalChanged(state); + } +} diff --git a/src/gui/networkinformation.h b/src/gui/networkinformation.h index 14ccdb07918..7b34217d386 100644 --- a/src/gui/networkinformation.h +++ b/src/gui/networkinformation.h @@ -20,6 +20,12 @@ namespace OCC { +/** + * @brief Wrapper class for QNetworkInformation + * + * This class is used instead of QNetworkInformation so we do not need to check for an instance, + * and to facilitate debugging by being able to force certain network states (i.e. captive portal). + */ class OWNCLOUDGUI_EXPORT NetworkInformation : public QObject { Q_OBJECT @@ -36,12 +42,22 @@ class OWNCLOUDGUI_EXPORT NetworkInformation : public QObject bool supports(Features features) const; + bool isForcedCaptivePortal() const; + void setForcedCaptivePortal(bool onoff); + bool isBehindCaptivePortal() const; + Q_SIGNALS: void isMeteredChanged(bool isMetered); void reachabilityChanged(NetworkInformation::Reachability reachability); + void isBehindCaptivePortalChanged(bool state); + +private Q_SLOTS: + void slotIsBehindCaptivePortalChanged(bool state); private: static NetworkInformation *_instance; + + bool _forcedCaptivePortal = false; }; } diff --git a/src/gui/owncloudgui.cpp b/src/gui/owncloudgui.cpp index f5c54c06705..36df95fc4e5 100644 --- a/src/gui/owncloudgui.cpp +++ b/src/gui/owncloudgui.cpp @@ -26,6 +26,7 @@ #include "folderwizard/folderwizard.h" #include "gui/accountsettings.h" #include "gui/commonstrings.h" +#include "gui/networkinformation.h" #include "guiutility.h" #include "libsync/theme.h" #include "logbrowser.h" @@ -652,6 +653,11 @@ void ownCloudGui::updateContextMenu() debugMenu->addAction(QStringLiteral("Crash now - OC_ENFORCE()"), _app, [] { OC_ENFORCE(false); }); debugMenu->addAction(QStringLiteral("Crash now - qFatal"), _app, [] { qFatal("la Qt fatale"); }); debugMenu->addAction(QStringLiteral("Restart now"), _app, [] { RestartManager::requestRestart(); }); + debugMenu->addSeparator(); + auto captivePortalCheckbox = debugMenu->addAction(QStringLiteral("Behind Captive Portal")); + captivePortalCheckbox->setCheckable(true); + captivePortalCheckbox->setChecked(NetworkInformation::instance()->isForcedCaptivePortal()); + connect(captivePortalCheckbox, &QAction::triggered, [](bool checked) { NetworkInformation::instance()->setForcedCaptivePortal(checked); }); } _contextMenu->addSeparator(); diff --git a/test/testutils/testutils.cpp b/test/testutils/testutils.cpp index ff45b069661..3c94e19709b 100644 --- a/test/testutils/testutils.cpp +++ b/test/testutils/testutils.cpp @@ -2,6 +2,7 @@ #include "common/checksums.h" #include "gui/accountmanager.h" +#include "gui/networkinformation.h" #include "libsync/creds/httpcredentials.h" #include @@ -27,6 +28,8 @@ namespace OCC { namespace TestUtils { TestUtilsPrivate::AccountStateRaii createDummyAccount() { + // ensure we have an instance of NetworkInformation + NetworkInformation::initialize(); // ensure we have an instance of folder man std::ignore = folderMan(); // don't use the account manager to create the account, it would try to use widgets