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

Stop synchronisation when behind a captive portal #11567

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/gui/CMakeLists.txt
Expand Up @@ -42,6 +42,7 @@ set(client_SRCS
ignorelisteditor.cpp
lockwatcher.cpp
logbrowser.cpp
networkinformation.cpp
networksettings.cpp
ocssharejob.cpp
openfilemanager.cpp
Expand Down
14 changes: 9 additions & 5 deletions src/gui/accountsettings.cpp
Expand Up @@ -30,6 +30,7 @@
#include "folderwizard/folderwizard.h"
#include "gui/accountmodalwidget.h"
#include "gui/models/models.h"
#include "gui/networkinformation.h"
#include "gui/qmlutils.h"
#include "gui/selectivesyncwidget.h"
#include "gui/spaces/spaceimageprovider.h"
Expand Down Expand Up @@ -401,7 +402,7 @@ void AccountSettings::slotEnableCurrentFolder(Folder *folder, bool terminate)

void AccountSettings::slotForceSyncCurrentFolder(Folder *folder)
{
if (Utility::internetConnectionIsMetered() && ConfigFile().pauseSyncWhenMetered()) {
if (NetworkInformation::instance()->isMetered() && ConfigFile().pauseSyncWhenMetered()) {
auto messageBox = new QMessageBox(QMessageBox::Question, tr("Internet connection is metered"),
tr("Synchronization is paused because the Internet connection is a metered connection"
"<p>Do you really want to force a Synchronization now?"),
Expand Down Expand Up @@ -459,9 +460,6 @@ void AccountSettings::slotAccountStateChanged()
.arg(Utility::escape(safeUrl.toString()));

switch (state) {
case AccountState::PausedDueToMetered:
showConnectionLabel(tr("Sync to %1 is paused due to metered internet connection.").arg(server));
break;
case AccountState::Connected: {
QStringList errors;
if (account->serverSupportLevel() != Account::ServerSupportLevel::Supported) {
Expand All @@ -484,7 +482,13 @@ 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 if (NetworkInformation::instance()->isMetered()) {
showConnectionLabel(tr("Sync to %1 is paused due to metered internet connection.").arg(server));
} else {
showConnectionLabel(tr("Connecting to: %1.").arg(server));
}
break;
case AccountState::ConfigurationError:
showConnectionLabel(tr("Server configuration error: %1.")
Expand Down
106 changes: 69 additions & 37 deletions src/gui/accountstate.cpp
Expand Up @@ -23,6 +23,7 @@
#include "libsync/creds/abstractcredentials.h"
#include "libsync/creds/httpcredentials.h"

#include "gui/networkinformation.h"
#include "gui/quotainfo.h"
#include "gui/settingsdialog.h"
#include "gui/spacemigration.h"
Expand All @@ -34,7 +35,6 @@
#include "theme.h"

#include <QFontMetrics>
#include <QNetworkInformation>
#include <QRandomGenerator>
#include <QSettings>
#include <QTimer>
Expand Down Expand Up @@ -113,42 +113,47 @@ AccountState::AccountState(AccountPtr account)
},
Qt::QueuedConnection);

#if QT_VERSION >= QT_VERSION_CHECK(6, 3, 0)
if (QNetworkInformation *qNetInfo = QNetworkInformation::instance()) {
connect(qNetInfo, &QNetworkInformation::reachabilityChanged, this, [this](QNetworkInformation::Reachability reachability) {
switch (reachability) {
case QNetworkInformation::Reachability::Online:
[[fallthrough]];
case QNetworkInformation::Reachability::Site:
[[fallthrough]];
case QNetworkInformation::Reachability::Unknown:
// the connection might not yet be established
QTimer::singleShot(0, this, [this] { checkConnectivity(false); });
break;
case QNetworkInformation::Reachability::Disconnected:
// explicitly set disconnected, this way a successful checkConnectivity call above will trigger a local discover
if (state() != State::SignedOut) {
setState(State::Disconnected);
}
[[fallthrough]];
case QNetworkInformation::Reachability::Local:
break;
connect(NetworkInformation::instance(), &NetworkInformation::reachabilityChanged, this, [this](NetworkInformation::Reachability reachability) {
switch (reachability) {
case NetworkInformation::Reachability::Online:
[[fallthrough]];
case NetworkInformation::Reachability::Site:
[[fallthrough]];
case NetworkInformation::Reachability::Unknown:
// the connection might not yet be established
QTimer::singleShot(0, this, [this] { checkConnectivity(false); });
break;
case NetworkInformation::Reachability::Disconnected:
// explicitly set disconnected, this way a successful checkConnectivity call above will trigger a local discover
if (state() != State::SignedOut) {
setState(State::Disconnected);
}
});
[[fallthrough]];
case NetworkInformation::Reachability::Local:
break;
}
});

connect(qNetInfo, &QNetworkInformation::isMeteredChanged, this, [this](bool isMetered) {
if (ConfigFile().pauseSyncWhenMetered()) {
if (state() == State::Connected && isMetered) {
qCInfo(lcAccountState) << "Network switched to a metered connection, setting account state to PausedDueToMetered";
setState(State::PausedDueToMetered);
} else if (state() == State::PausedDueToMetered && !isMetered) {
qCInfo(lcAccountState) << "Network switched to a NON-metered connection, setting account state to Connected";
setState(State::Connected);
}
connect(NetworkInformation::instance(), &NetworkInformation::isMeteredChanged, this, [this](bool isMetered) {
if (ConfigFile().pauseSyncWhenMetered()) {
if (state() == State::Connected && isMetered) {
qCInfo(lcAccountState) << "Network switched to a metered connection, setting account state to PausedDueToMetered";
setState(State::Connecting);
} else if (state() == State::Connecting && !isMetered) {
qCInfo(lcAccountState) << "Network switched to a NON-metered connection, setting account state to Connected";
setState(State::Connected);
}
});
}
#endif
}
});

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`.
// The call is also made for when we "go behind" a captive portal. That ensures that not
// only the status is set to `Connecting`, but also makes the UI show that syncing is paused.
QTimer::singleShot(0, this, [this] { checkConnectivity(false); });
erikjv marked this conversation as resolved.
Show resolved Hide resolved
});

// as a fallback and to recover after server issues we also poll
auto timer = new QTimer(this);
timer->setInterval(ConnectionValidator::DefaultCallingInterval);
Expand All @@ -169,6 +174,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?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. It might depend on how long we were blocked, 10s or 20h...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not sure what to do with this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets clear it.

_queueGuard.unblock();
}
});
if (NetworkInformation::instance()->isBehindCaptivePortal()) {
_queueGuard.block();
} else {
// TODO: empty queue?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this unblock has any use?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we be blocked during the construction of the class?

_queueGuard.unblock();
}
}

AccountState::~AccountState() { }
Expand Down Expand Up @@ -242,8 +263,11 @@ void AccountState::setState(State state)
_connectionValidator->deleteLater();
_connectionValidator.clear();
checkConnectivity();
} else if (_state == Connected && Utility::internetConnectionIsMetered() && ConfigFile().pauseSyncWhenMetered()) {
_state = PausedDueToMetered;
} else if (_state == Connected) {
if ((NetworkInformation::instance()->isMetered() && ConfigFile().pauseSyncWhenMetered())
|| NetworkInformation::instance()->isBehindCaptivePortal()) {
_state = Connecting;
}
}
}

Expand Down Expand Up @@ -303,7 +327,7 @@ void AccountState::signIn()

bool AccountState::isConnected() const
{
return _state == Connected || _state == PausedDueToMetered;
return _state == Connected;
}

void AccountState::tagLastSuccessfullETagRequest(const QDateTime &tp)
Expand Down Expand Up @@ -365,6 +389,9 @@ void AccountState::checkConnectivity(bool blockJobs)
this, &AccountState::slotConnectionValidatorResult);

connect(_connectionValidator, &ConnectionValidator::sslErrors, this, [blockJobs, this](const QList<QSslError> &errors) {
if (NetworkInformation::instance()->isBehindCaptivePortal()) {
return;
}
if (!_tlsDialog) {
// ignore errors for already accepted certificates
auto filteredErrors = _account->accessManager()->filterSslErrors(errors);
Expand Down Expand Up @@ -480,6 +507,7 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta
setState(Connected);
break;
case ConnectionValidator::Undefined:
[[fallthrough]];
case ConnectionValidator::NotConfigured:
setState(Disconnected);
break;
Expand All @@ -495,6 +523,7 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta
setState(NetworkError);
break;
case ConnectionValidator::CredentialsWrong:
[[fallthrough]];
case ConnectionValidator::CredentialsNotReady:
slotInvalidCredentials();
break;
Expand All @@ -512,6 +541,9 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta
case ConnectionValidator::Timeout:
setState(NetworkError);
break;
case ConnectionValidator::CaptivePortal:
setState(Connecting);
break;
}
}

Expand Down
5 changes: 0 additions & 5 deletions src/gui/accountstate.h
Expand Up @@ -79,11 +79,6 @@ class OWNCLOUDGUI_EXPORT AccountState : public QObject
/// We are currently asking the user for credentials
AskingCredentials,

/// We are on a metered internet connection, and the user preference
/// is to pause syncing in this case. This state is entered from and
/// left to a `Connected` state.
PausedDueToMetered,

Connecting
};
Q_ENUM(State)
Expand Down
2 changes: 0 additions & 2 deletions src/gui/application.cpp
Expand Up @@ -48,8 +48,6 @@
#include <QMessageBox>
#include <QPushButton>

#include <QNetworkInformation>

namespace OCC {

Q_LOGGING_CATEGORY(lcApplication, "gui.application", QtInfoMsg)
Expand Down
5 changes: 3 additions & 2 deletions src/gui/connectionvalidator.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)) {
Expand Down
5 changes: 3 additions & 2 deletions src/gui/connectionvalidator.h
Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion src/gui/folderman.cpp
Expand Up @@ -20,6 +20,7 @@
#include "common/asserts.h"
#include "configfile.h"
#include "folder.h"
#include "gui/networkinformation.h"
#include "guiutility.h"
#include "libsync/syncengine.h"
#include "lockwatcher.h"
Expand Down Expand Up @@ -73,7 +74,9 @@ void TrayOverallStatusResult::addResult(Folder *f)
lastSyncDone = time;
}

auto status = f->syncPaused() || f->accountState()->state() == AccountState::PausedDueToMetered ? SyncResult::Paused : f->syncResult().status();
auto status = f->syncPaused() || NetworkInformation::instance()->isBehindCaptivePortal() || NetworkInformation::instance()->isMetered()
? SyncResult::Paused
: f->syncResult().status();
if (status == SyncResult::Undefined) {
status = SyncResult::Problem;
}
Expand Down
3 changes: 2 additions & 1 deletion src/gui/folderstatusmodel.cpp
Expand Up @@ -17,6 +17,7 @@
#include "accountstate.h"
#include "common/asserts.h"
#include "folderman.h"
#include "gui/networkinformation.h"
#include "gui/quotainfo.h"
#include "theme.h"

Expand Down Expand Up @@ -50,7 +51,7 @@ namespace {
auto status = f->syncResult();
if (!f->accountState()->isConnected()) {
status.setStatus(SyncResult::Status::Offline);
} else if (f->syncPaused() || f->accountState()->state() == AccountState::PausedDueToMetered) {
} else if (f->syncPaused() || NetworkInformation::instance()->isBehindCaptivePortal() || NetworkInformation::instance()->isMetered()) {
status.setStatus(SyncResult::Status::Paused);
}
return Theme::instance()->syncStateIconName(status);
Expand Down
10 changes: 0 additions & 10 deletions src/gui/guiutility.cpp
Expand Up @@ -22,7 +22,6 @@
#include <QDesktopServices>
#include <QLoggingCategory>
#include <QMessageBox>
#include <QNetworkInformation>
#include <QQuickWidget>
#include <QUrlQuery>

Expand Down Expand Up @@ -96,15 +95,6 @@ QString Utility::vfsFreeSpaceActionText()
return QCoreApplication::translate("utility", "Free up local space");
}

bool Utility::internetConnectionIsMetered()
{
if (auto *qNetInfo = QNetworkInformation::instance()) {
return qNetInfo->isMetered();
}

return false;
}

void Utility::markDirectoryAsSyncRoot(const QString &path, const QUuid &accountUuid)
{
Q_ASSERT(getDirectorySyncRootMarkings(path).first.isEmpty());
Expand Down
2 changes: 0 additions & 2 deletions src/gui/guiutility.h
Expand Up @@ -52,8 +52,6 @@ namespace Utility {

QString socketApiSocketPath();

bool internetConnectionIsMetered();

OWNCLOUDGUI_EXPORT void markDirectoryAsSyncRoot(const QString &path, const QUuid &accountUuid);
std::pair<QString, QUuid> getDirectorySyncRootMarkings(const QString &path);
void unmarkDirectoryAsSyncRoot(const QString &path);
Expand Down