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

Don't follow redirects besides in the connection validator #8253

Merged
merged 15 commits into from
Dec 9, 2020
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
58 changes: 53 additions & 5 deletions src/gui/accountstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@
* for more details.
*/

#include "application.h"
#include "accountstate.h"
#include "accountmanager.h"
#include "account.h"
#include "creds/abstractcredentials.h"
#include "creds/httpcredentials.h"
#include "logger.h"
#include "configfile.h"
#include "settingsdialog.h"

#include <QMessageBox>
#include <QSettings>
#include <QTimer>
#include <qfontmetrics.h>
Expand All @@ -28,6 +31,39 @@ namespace OCC {

Q_LOGGING_CATEGORY(lcAccountState, "gui.account.state", QtInfoMsg)

void AccountState::updateUrlDialog(AccountPtr account, const QUrl &newUrl, std::function<void()> callback)
{
auto accept = [=](bool accepted) {
if (accepted) {
account->setUrl(newUrl);
Q_EMIT account->wantsAccountSaved(account.data());
}
if (callback) {
callback();
}
};
// the urls are identical, previous versions of owncloud cropped the /
if (newUrl.path() == QLatin1Char('/')) {
auto tmp = newUrl;
tmp.setPath(QString());
if (tmp == account->url()) {
// silently accept the /
accept(true);
return;
}
}
auto diag = new QMessageBox(QMessageBox::Warning, tr("Url update requested for %1").arg(account->displayName()),
tr("The url for %1 changed from %2 to %3, do you want to accept the changed url?").arg(account->displayName(), account->url().toString(), newUrl.toString()),
QMessageBox::NoButton, ocApp()->gui()->settingsDialog());
diag->setAttribute(Qt::WA_DeleteOnClose);
auto yes = diag->addButton(tr("Change url permanently to %1").arg(newUrl.toString()), QMessageBox::AcceptRole);
diag->addButton(tr("Reject"), QMessageBox::RejectRole);
QObject::connect(diag, &QMessageBox::finished, account.data(), [diag, yes, accept] {
accept(diag->clickedButton() == yes);
});
diag->show();
}

AccountState::AccountState(AccountPtr account)
: QObject()
, _account(account)
Expand All @@ -44,6 +80,13 @@ AccountState::AccountState(AccountPtr account)
this, &AccountState::slotCredentialsFetched);
connect(account.data(), &Account::credentialsAsked,
this, &AccountState::slotCredentialsAsked);
connect(account.data(), &Account::unknownConnectionState,
this, [this] {
checkConnectivity(true);
});
connect(account.data(), &Account::requestUrlUpdate, this, [this](const QUrl &newUrl) {
updateUrlDialog(_account, newUrl, [this] { checkConnectivity(); });
});
}

AccountState::~AccountState()
Expand Down Expand Up @@ -171,8 +214,9 @@ void AccountState::tagLastSuccessfullETagRequest(const QDateTime &tp)
_timeOfLastETagCheck = tp;
}

void AccountState::checkConnectivity()
void AccountState::checkConnectivity(bool verifyOnly)
{
qCWarning(lcAccountState) << "checkConnectivity verifyOnly:" << verifyOnly;
if (isSignedOut() || _waitingForNewCredentials) {
return;
}
Expand All @@ -194,7 +238,7 @@ void AccountState::checkConnectivity()
// if the last successful etag check job is not so long ago.
const auto polltime = std::chrono::duration_cast<std::chrono::seconds>(ConfigFile().remotePollInterval());
const auto elapsed = _timeOfLastETagCheck.secsTo(QDateTime::currentDateTimeUtc());
if (isConnected() && _timeOfLastETagCheck.isValid()
if (!verifyOnly && isConnected() && _timeOfLastETagCheck.isValid()
&& elapsed <= polltime.count()) {
qCDebug(lcAccountState) << account()->displayName() << "The last ETag check succeeded within the last " << polltime.count() << "s (" << elapsed << "s). No connection check needed!";
return;
Expand All @@ -207,7 +251,11 @@ void AccountState::checkConnectivity()
if (isConnected()) {
// Use a small authed propfind as a minimal ping when we're
// already connected.
conValidator->checkAuthentication();
if (verifyOnly) {
conValidator->checkServer();
} else {
conValidator->checkServerAndUpdate();
}
} else {
// Check the server and then the auth.

Expand All @@ -224,7 +272,7 @@ void AccountState::checkConnectivity()
// ssl config that does not have a sensible certificate chain.
account()->setSslConfiguration(QSslConfiguration());
//#endif
conValidator->checkServerAndAuth();
conValidator->checkServerAndUpdate();
}
}

Expand All @@ -243,7 +291,7 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta
qCInfo(lcAccountState) << "AccountState reconnection: delaying for"
<< _maintenanceToConnectedDelay << "ms";
_timeSinceMaintenanceOver.start();
QTimer::singleShot(_maintenanceToConnectedDelay + 100, this, &AccountState::checkConnectivity);
QTimer::singleShot(_maintenanceToConnectedDelay + 100, this, [this] { AccountState::checkConnectivity(false); });
return;
} else if (_timeSinceMaintenanceOver.elapsed() < _maintenanceToConnectedDelay) {
qCInfo(lcAccountState) << "AccountState reconnection: only"
Expand Down
3 changes: 2 additions & 1 deletion src/gui/accountstate.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class AccountState : public QObject, public QSharedData
Q_PROPERTY(AccountPtr account MEMBER _account)

public:
static void updateUrlDialog(AccountPtr account, const QUrl &url, std::function<void()> callback = {});
enum State {
/// Not even attempting to connect, most likely because the
/// user explicitly signed out or cancelled a credential dialog.
Expand Down Expand Up @@ -135,7 +136,7 @@ class AccountState : public QObject, public QSharedData
public slots:
/// Triggers a ping to the server to update state and
/// connection status and errors.
void checkConnectivity();
void checkConnectivity(bool verifyOnly = false);

private:
void setState(State state);
Expand Down
30 changes: 18 additions & 12 deletions src/gui/connectionvalidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,16 @@ static qint64 timeoutToUseMsec = qMax(1000, ConnectionValidator::DefaultCallingI
ConnectionValidator::ConnectionValidator(AccountPtr account, QObject *parent)
: QObject(parent)
, _account(account)
, _isCheckingServerAndAuth(false)
{
}

void ConnectionValidator::checkServerAndAuth()
void ConnectionValidator::checkServer()
{
_updateConfig = false;
checkServerAndUpdate();
}

void ConnectionValidator::checkServerAndUpdate()
{
if (!_account) {
_errors << tr("No ownCloud account configured");
Expand All @@ -49,8 +54,6 @@ void ConnectionValidator::checkServerAndAuth()
}
qCDebug(lcConnectionValidator) << "Checking server and authentication";

_isCheckingServerAndAuth = true;

// Lookup system proxy in a thread https://github.com/owncloud/client/issues/2993
if (ClientProxy::isUsingSystemDefault()) {
qCDebug(lcConnectionValidator) << "Trying to look up system proxy";
Expand Down Expand Up @@ -86,7 +89,6 @@ void ConnectionValidator::slotCheckServerAndAuth()
{
CheckServerJob *checkJob = new CheckServerJob(_account, this);
checkJob->setTimeout(timeoutToUseMsec);
checkJob->setIgnoreCredentialFailure(true);
connect(checkJob, &CheckServerJob::instanceFound, this, &ConnectionValidator::slotStatusFound);
connect(checkJob, &CheckServerJob::instanceNotFound, this, &ConnectionValidator::slotNoStatusFound);
connect(checkJob, &CheckServerJob::timeout, this, &ConnectionValidator::slotJobTimeout);
Expand All @@ -108,9 +110,10 @@ void ConnectionValidator::slotStatusFound(const QUrl &url, const QJsonObject &in

// Update server url in case of redirection
if (_account->url() != url) {
qCInfo(lcConnectionValidator()) << "status.php was redirected to" << url.toString();
_account->setUrl(url);
_account->wantsAccountSaved(_account.data());
qCInfo(lcConnectionValidator()) << "status.php was redirected to" << url.toString() << "asking user to accept and abort for now";
TheOneRing marked this conversation as resolved.
Show resolved Hide resolved
Q_EMIT _account->requestUrlUpdate(url);
reportResult(StatusNotFound);
return;
}

if (!serverVersion.isEmpty() && !setAndCheckServerVersion(serverVersion)) {
Expand All @@ -136,9 +139,10 @@ void ConnectionValidator::slotNoStatusFound(QNetworkReply *reply)
if (reply->error() == QNetworkReply::SslHandshakeFailedError) {
reportResult(SslError);
return;
}

if (!_account->credentials()->stillValid(reply)) {
} else if (reply->error() == QNetworkReply::TooManyRedirectsError) {
reportResult(MaintenanceMode);
return;
} else if (!_account->credentials()->stillValid(reply)) {
// Note: Why would this happen on a status.php request?
_errors.append(tr("Authentication error: Either username or password are wrong."));
} else {
Expand Down Expand Up @@ -170,6 +174,7 @@ void ConnectionValidator::checkAuthentication()
// continue in slotAuthCheck here :-)
qCDebug(lcConnectionValidator) << "# Check whether authenticated propfind works.";
PropfindJob *job = new PropfindJob(_account, "/", this);
job->setAuthenticationJob(true); // don't retry
job->setTimeout(timeoutToUseMsec);
job->setProperties(QList<QByteArray>() << "getlastmodified");
connect(job, &PropfindJob::result, this, &ConnectionValidator::slotAuthSuccess);
Expand Down Expand Up @@ -209,10 +214,11 @@ void ConnectionValidator::slotAuthFailed(QNetworkReply *reply)
void ConnectionValidator::slotAuthSuccess()
{
_errors.clear();
if (!_isCheckingServerAndAuth) {
if (!_updateConfig) {
reportResult(Connected);
return;
}

checkServerCapabilities();
}

Expand Down
10 changes: 5 additions & 5 deletions src/gui/connectionvalidator.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,16 @@ class ConnectionValidator : public QObject

public slots:
/// Checks the server and the authentication.
void checkServerAndAuth();
void checkServer();
void checkServerAndUpdate();
void systemProxyLookupDone(const QNetworkProxy &proxy);

/// Checks authentication only.
void checkAuthentication();

signals:
void connectionResult(ConnectionValidator::Status status, const QStringList &errors);

protected slots:
/// Checks authentication only.
void checkAuthentication();
void slotCheckServerAndAuth();

void slotStatusFound(const QUrl &url, const QJsonObject &info);
Expand Down Expand Up @@ -139,7 +139,7 @@ protected slots:

QStringList _errors;
AccountPtr _account;
bool _isCheckingServerAndAuth;
bool _updateConfig = true;
};
}

Expand Down
Loading