From 1c0d80c20da72cb9de5b134648ed4cb6b5aa86e7 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Thu, 7 Sep 2017 14:58:45 +0200 Subject: [PATCH] Use DetermineAuthTypeJob in HttpCredentials * Move it to networkjobs * Minor adjustments to its logic * Fixes redirect handling for oauth/basic http auth check #6003 --- src/gui/creds/httpcredentialsgui.cpp | 20 +++---- src/gui/owncloudsetupwizard.cpp | 58 +------------------ src/gui/owncloudsetupwizard.h | 19 ------ src/gui/wizard/owncloudoauthcredspage.cpp | 2 +- src/gui/wizard/owncloudsetuppage.cpp | 8 +-- src/gui/wizard/owncloudsetuppage.h | 4 +- src/gui/wizard/owncloudwizard.cpp | 8 +-- src/gui/wizard/owncloudwizard.h | 3 +- src/gui/wizard/owncloudwizardcommon.h | 6 -- src/libsync/creds/httpcredentials.cpp | 22 +++---- src/libsync/creds/httpcredentials.h | 4 ++ src/libsync/networkjobs.cpp | 70 +++++++++++++++++++++++ src/libsync/networkjobs.h | 26 +++++++++ 13 files changed, 136 insertions(+), 114 deletions(-) diff --git a/src/gui/creds/httpcredentialsgui.cpp b/src/gui/creds/httpcredentialsgui.cpp index e65554f49..98e4df907 100644 --- a/src/gui/creds/httpcredentialsgui.cpp +++ b/src/gui/creds/httpcredentialsgui.cpp @@ -22,6 +22,7 @@ #include "creds/httpcredentialsgui.h" #include "theme.h" #include "account.h" +#include "networkjobs.h" #include #include "asserts.h" @@ -39,15 +40,11 @@ void HttpCredentialsGui::askFromUser() void HttpCredentialsGui::askFromUserAsync() { - _password = QString(); // So our QNAM does not add any auth - - // First, we will send a call to the webdav endpoint to check what kind of auth we need. - auto reply = _account->sendRequest("GET", _account->davUrl()); - QTimer::singleShot(30 * 1000, reply, &QNetworkReply::abort); - QObject::connect(reply, &QNetworkReply::finished, this, [this, reply] { - reply->deleteLater(); - if (reply->rawHeader("WWW-Authenticate").contains("Bearer ")) { - // OAuth + // First, we will check what kind of auth we need. + auto job = new DetermineAuthTypeJob(_account->sharedFromThis(), this); + job->setTimeout(30 * 1000); + QObject::connect(job, &DetermineAuthTypeJob::authType, this, [this](DetermineAuthTypeJob::AuthType type) { + if (type == DetermineAuthTypeJob::OAuth) { _asyncAuth.reset(new OAuth(_account, this)); _asyncAuth->_expectedUser = _user; connect(_asyncAuth.data(), &OAuth::result, @@ -56,15 +53,16 @@ void HttpCredentialsGui::askFromUserAsync() this, &HttpCredentialsGui::authorisationLinkChanged); _asyncAuth->start(); emit authorisationLinkChanged(); - } else if (reply->error() == QNetworkReply::AuthenticationRequiredError) { + } else if (type == DetermineAuthTypeJob::Basic) { // Show the dialog // We will re-enter the event loop, so better wait the next iteration QMetaObject::invokeMethod(this, "showDialog", Qt::QueuedConnection); } else { - // Network error? + // Network error? Unsupported auth type? emit asked(); } }); + job->start(); } void HttpCredentialsGui::asyncAuthResult(OAuth::Result r, const QString &user, diff --git a/src/gui/owncloudsetupwizard.cpp b/src/gui/owncloudsetupwizard.cpp index 3ee02c73a..4b2886e39 100644 --- a/src/gui/owncloudsetupwizard.cpp +++ b/src/gui/owncloudsetupwizard.cpp @@ -207,8 +207,8 @@ void OwncloudSetupWizard::slotOwnCloudFoundAuth(const QUrl &url, const QJsonObje DetermineAuthTypeJob *job = new DetermineAuthTypeJob(_ocWizard->account(), this); job->setIgnoreCredentialFailure(true); - connect(job, SIGNAL(authType(WizardCommon::AuthType)), - _ocWizard, SLOT(setAuthType(WizardCommon::AuthType))); + connect(job, &DetermineAuthTypeJob::authType, + _ocWizard, &OwncloudWizard::setAuthType); job->start(); } @@ -600,58 +600,4 @@ AccountState *OwncloudSetupWizard::applyAccountChanges() return newState; } - -DetermineAuthTypeJob::DetermineAuthTypeJob(AccountPtr account, QObject *parent) - : AbstractNetworkJob(account, QString(), parent) - , _redirects(0) -{ - // This job implements special redirect handling to detect redirections - // to pages that are indicative of Shibboleth-using servers. Hence we - // disable the standard job redirection handling here. - _followRedirects = false; -} - -void DetermineAuthTypeJob::start() -{ - sendRequest("GET", account()->davUrl()); - AbstractNetworkJob::start(); -} - -bool DetermineAuthTypeJob::finished() -{ - QUrl redirection = reply()->attribute(QNetworkRequest::RedirectionTargetAttribute).toUrl(); - qCDebug(lcWizard) << redirection.toString(); - if (_redirects >= maxRedirects()) { - redirection.clear(); - } - if ((reply()->error() == QNetworkReply::AuthenticationRequiredError) || redirection.isEmpty()) { - if (reply()->rawHeader("WWW-Authenticate").contains("Bearer ")) { - emit authType(WizardCommon::OAuth); - } else { - emit authType(WizardCommon::HttpCreds); - } - } else if (redirection.toString().endsWith(account()->davPath())) { - // do a new run - _redirects++; - resetTimeout(); - sendRequest("GET", redirection); - return false; // don't discard - } else { -#ifndef NO_SHIBBOLETH - QRegExp shibbolethyWords("SAML|wayf"); - - shibbolethyWords.setCaseSensitivity(Qt::CaseInsensitive); - if (redirection.toString().contains(shibbolethyWords)) { - emit authType(WizardCommon::Shibboleth); - } else -#endif - { - // TODO: Send an error. - // eh? - emit authType(WizardCommon::HttpCreds); - } - } - return true; -} - } // namespace OCC diff --git a/src/gui/owncloudsetupwizard.h b/src/gui/owncloudsetupwizard.h index 39f401db6..28fa884fa 100644 --- a/src/gui/owncloudsetupwizard.h +++ b/src/gui/owncloudsetupwizard.h @@ -33,25 +33,6 @@ class AccountState; class OwncloudWizard; -/** - * @brief The DetermineAuthTypeJob class - * @ingroup gui - */ -class DetermineAuthTypeJob : public AbstractNetworkJob -{ - Q_OBJECT -public: - explicit DetermineAuthTypeJob(AccountPtr account, QObject *parent = 0); - void start() Q_DECL_OVERRIDE; -signals: - void authType(WizardCommon::AuthType); -private slots: - bool finished() Q_DECL_OVERRIDE; - -private: - int _redirects; -}; - /** * @brief The OwncloudSetupWizard class * @ingroup gui diff --git a/src/gui/wizard/owncloudoauthcredspage.cpp b/src/gui/wizard/owncloudoauthcredspage.cpp index a4bf5988e..6d9ed4c75 100644 --- a/src/gui/wizard/owncloudoauthcredspage.cpp +++ b/src/gui/wizard/owncloudoauthcredspage.cpp @@ -76,7 +76,7 @@ void OwncloudOAuthCredsPage::asyncAuthResult(OAuth::Result r, const QString &use /* OAuth not supported (can't open browser), fallback to HTTP credentials */ OwncloudWizard *ocWizard = qobject_cast(wizard()); ocWizard->back(); - ocWizard->setAuthType(WizardCommon::HttpCreds); + ocWizard->setAuthType(DetermineAuthTypeJob::Basic); break; } case OAuth::Error: diff --git a/src/gui/wizard/owncloudsetuppage.cpp b/src/gui/wizard/owncloudsetuppage.cpp index 271c943a2..39afeecfc 100644 --- a/src/gui/wizard/owncloudsetuppage.cpp +++ b/src/gui/wizard/owncloudsetuppage.cpp @@ -40,7 +40,7 @@ OwncloudSetupPage::OwncloudSetupPage(QWidget *parent) , _ocUser() , _authTypeKnown(false) , _checking(false) - , _authType(WizardCommon::HttpCreds) + , _authType(DetermineAuthTypeJob::Basic) , _progressIndi(new QProgressIndicator(this)) { _ui.setupUi(this); @@ -201,9 +201,9 @@ bool OwncloudSetupPage::urlHasChanged() int OwncloudSetupPage::nextId() const { - if (_authType == WizardCommon::HttpCreds) { + if (_authType == DetermineAuthTypeJob::Basic) { return WizardCommon::Page_HttpCreds; - } else if (_authType == WizardCommon::OAuth) { + } else if (_authType == DetermineAuthTypeJob::OAuth) { return WizardCommon::Page_OAuthCreds; } else { return WizardCommon::Page_ShibbolethCreds; @@ -235,7 +235,7 @@ bool OwncloudSetupPage::validatePage() } } -void OwncloudSetupPage::setAuthType(WizardCommon::AuthType type) +void OwncloudSetupPage::setAuthType(DetermineAuthTypeJob::AuthType type) { _authTypeKnown = true; _authType = type; diff --git a/src/gui/wizard/owncloudsetuppage.h b/src/gui/wizard/owncloudsetuppage.h index accd39989..13efa806b 100644 --- a/src/gui/wizard/owncloudsetuppage.h +++ b/src/gui/wizard/owncloudsetuppage.h @@ -53,7 +53,7 @@ public: QString localFolder() const; void setRemoteFolder(const QString &remoteFolder); void setMultipleFoldersExist(bool exist); - void setAuthType(WizardCommon::AuthType type); + void setAuthType(DetermineAuthTypeJob::AuthType type); public slots: void setErrorString(const QString &, bool retryHTTPonly); @@ -80,7 +80,7 @@ private: bool _authTypeKnown; bool _checking; bool _multipleFoldersExist; - WizardCommon::AuthType _authType; + DetermineAuthTypeJob::AuthType _authType; QProgressIndicator *_progressIndi; QButtonGroup *_selectiveSyncButtons; diff --git a/src/gui/wizard/owncloudwizard.cpp b/src/gui/wizard/owncloudwizard.cpp index 9fb20ced6..a1c24bbd4 100644 --- a/src/gui/wizard/owncloudwizard.cpp +++ b/src/gui/wizard/owncloudwizard.cpp @@ -167,17 +167,17 @@ void OwncloudWizard::successfulStep() next(); } -void OwncloudWizard::setAuthType(WizardCommon::AuthType type) +void OwncloudWizard::setAuthType(DetermineAuthTypeJob::AuthType type) { _setupPage->setAuthType(type); #ifndef NO_SHIBBOLETH - if (type == WizardCommon::Shibboleth) { + if (type == DetermineAuthTypeJob::Shibboleth) { _credentialsPage = _shibbolethCredsPage; } else #endif - if (type == WizardCommon::OAuth) { + if (type == DetermineAuthTypeJob::OAuth) { _credentialsPage = _browserCredsPage; - } else { + } else { // try Basic auth even for "Unknown" _credentialsPage = _httpCredsPage; } next(); diff --git a/src/gui/wizard/owncloudwizard.h b/src/gui/wizard/owncloudwizard.h index 78f5bb444..e82817a03 100644 --- a/src/gui/wizard/owncloudwizard.h +++ b/src/gui/wizard/owncloudwizard.h @@ -21,6 +21,7 @@ #include #include +#include "networkjobs.h" #include "wizard/owncloudwizardcommon.h" #include "accountfwd.h" @@ -75,7 +76,7 @@ public: QSslCertificate _clientSslCertificate; public slots: - void setAuthType(WizardCommon::AuthType type); + void setAuthType(DetermineAuthTypeJob::AuthType type); void setRemoteFolder(const QString &); void appendToConfigurationLog(const QString &msg, LogType type = LogParagraph); void slotCurrentPageChanged(int); diff --git a/src/gui/wizard/owncloudwizardcommon.h b/src/gui/wizard/owncloudwizardcommon.h index eaad00704..c55ed04fd 100644 --- a/src/gui/wizard/owncloudwizardcommon.h +++ b/src/gui/wizard/owncloudwizardcommon.h @@ -28,12 +28,6 @@ namespace WizardCommon { QString subTitleTemplate(); void initErrorLabel(QLabel *errorLabel); - enum AuthType { - HttpCreds, - Shibboleth, - OAuth - }; - enum SyncMode { SelectiveMode, BoxMode diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp index 53215de2a..b82588f84 100644 --- a/src/libsync/creds/httpcredentials.cpp +++ b/src/libsync/creds/httpcredentials.cpp @@ -58,18 +58,20 @@ protected: QNetworkReply *createRequest(Operation op, const QNetworkRequest &request, QIODevice *outgoingData) Q_DECL_OVERRIDE { QNetworkRequest req(request); - if (_cred && !_cred->password().isEmpty()) { - if (_cred->isUsingOAuth()) { - req.setRawHeader("Authorization", "Bearer " + _cred->password().toUtf8()); - } else { - QByteArray credHash = QByteArray(_cred->user().toUtf8() + ":" + _cred->password().toUtf8()).toBase64(); + if (!req.attribute(HttpCredentials::DontAddCredentialsAttribute).toBool()) { + if (_cred && !_cred->password().isEmpty()) { + if (_cred->isUsingOAuth()) { + req.setRawHeader("Authorization", "Bearer " + _cred->password().toUtf8()); + } else { + QByteArray credHash = QByteArray(_cred->user().toUtf8() + ":" + _cred->password().toUtf8()).toBase64(); + req.setRawHeader("Authorization", "Basic " + credHash); + } + } else if (!request.url().password().isEmpty()) { + // Typically the requests to get or refresh the OAuth access token. The client + // credentials are put in the URL from the code making the request. + QByteArray credHash = request.url().userInfo().toUtf8().toBase64(); req.setRawHeader("Authorization", "Basic " + credHash); } - } else if (!request.url().password().isEmpty()) { - // Typically the requests to get or refresh the OAuth access token. The client - // credentials are put in the URL from the code making the request. - QByteArray credHash = request.url().userInfo().toUtf8().toBase64(); - req.setRawHeader("Authorization", "Basic " + credHash); } if (_cred && !_cred->_clientSslKey.isNull() && !_cred->_clientSslCertificate.isNull()) { diff --git a/src/libsync/creds/httpcredentials.h b/src/libsync/creds/httpcredentials.h index df3bcdabc..0dc7ad732 100644 --- a/src/libsync/creds/httpcredentials.h +++ b/src/libsync/creds/httpcredentials.h @@ -19,6 +19,7 @@ #include #include #include +#include #include "creds/abstractcredentials.h" class QNetworkReply; @@ -75,6 +76,9 @@ class OWNCLOUDSYNC_EXPORT HttpCredentials : public AbstractCredentials friend class HttpCredentialsAccessManager; public: + /// Don't add credentials if this is set on a QNetworkRequest + static constexpr QNetworkRequest::Attribute DontAddCredentialsAttribute = QNetworkRequest::User; + explicit HttpCredentials(); HttpCredentials(const QString &user, const QString &password, const QSslCertificate &certificate = QSslCertificate(), const QSslKey &key = QSslKey()); diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp index 613098dda..304bb839d 100644 --- a/src/libsync/networkjobs.cpp +++ b/src/libsync/networkjobs.cpp @@ -36,6 +36,7 @@ #include "owncloudpropagator.h" #include "creds/abstractcredentials.h" +#include "creds/httpcredentials.h" namespace OCC { @@ -47,6 +48,7 @@ Q_LOGGING_CATEGORY(lcAvatarJob, "sync.networkjob.avatar", QtInfoMsg) Q_LOGGING_CATEGORY(lcMkColJob, "sync.networkjob.mkcol", QtInfoMsg) Q_LOGGING_CATEGORY(lcProppatchJob, "sync.networkjob.proppatch", QtInfoMsg) Q_LOGGING_CATEGORY(lcJsonApiJob, "sync.networkjob.jsonapi", QtInfoMsg) +Q_LOGGING_CATEGORY(lcDetermineAuthTypeJob, "sync.networkjob.determineauthtype", QtInfoMsg) RequestEtagJob::RequestEtagJob(AccountPtr account, const QString &path, QObject *parent) : AbstractNetworkJob(account, path, parent) @@ -798,4 +800,72 @@ bool JsonApiJob::finished() return true; } +DetermineAuthTypeJob::DetermineAuthTypeJob(AccountPtr account, QObject *parent) + : AbstractNetworkJob(account, QString(), parent) + , _redirects(0) +{ + // This job implements special redirect handling to detect redirections + // to pages that are indicative of Shibboleth-using servers. Hence we + // disable the standard job redirection handling here. + _followRedirects = false; +} + +void DetermineAuthTypeJob::start() +{ + send(account()->davUrl()); + AbstractNetworkJob::start(); +} + +bool DetermineAuthTypeJob::finished() +{ + QUrl redirection = reply()->attribute(QNetworkRequest::RedirectionTargetAttribute).toUrl(); + if (_redirects >= maxRedirects()) { + redirection.clear(); + } + + auto authChallenge = reply()->rawHeader("WWW-Authenticate").toLower(); + if (redirection.isEmpty()) { + if (authChallenge.contains("bearer ")) { + emit authType(OAuth); + } else if (!authChallenge.isEmpty()) { + emit authType(Basic); + } else { + // This is also where we end up in case of network error. + emit authType(Unknown); + } + } else if (redirection.toString().endsWith(account()->davPath())) { + // do a new run + _redirects++; + resetTimeout(); + send(redirection); + qCDebug(lcDetermineAuthTypeJob()) << "Redirected to:" << redirection.toString(); + return false; // don't discard + } else { +#ifndef NO_SHIBBOLETH + QRegExp shibbolethyWords("SAML|wayf"); + shibbolethyWords.setCaseSensitivity(Qt::CaseInsensitive); + if (redirection.toString().contains(shibbolethyWords)) { + emit authType(Shibboleth); + } else +#endif + { + // We got redirected to an address that doesn't look like shib + // and also doesn't have the davPath. Give up. + qCWarning(lcDetermineAuthTypeJob()) << account()->davUrl() + << "was redirected to the incompatible address" + << redirection.toString(); + emit authType(Unknown); + } + } + return true; +} + +void DetermineAuthTypeJob::send(const QUrl &url) +{ + QNetworkRequest req; + // Prevent HttpCredentialsAccessManager from setting an Authorization header. + req.setAttribute(HttpCredentials::DontAddCredentialsAttribute, true); + sendRequest("GET", url, req); +} + } // namespace OCC diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h index 8929391f2..fb54e95ba 100644 --- a/src/libsync/networkjobs.h +++ b/src/libsync/networkjobs.h @@ -330,6 +330,32 @@ private: QList> _additionalParams; }; +/** + * @brief Checks with auth type to use for a server + * @ingroup libsync + */ +class OWNCLOUDSYNC_EXPORT DetermineAuthTypeJob : public AbstractNetworkJob +{ + Q_OBJECT +public: + enum AuthType { + Unknown, + Basic, + OAuth, + Shibboleth + }; + + explicit DetermineAuthTypeJob(AccountPtr account, QObject *parent = 0); + void start() Q_DECL_OVERRIDE; +signals: + void authType(AuthType); +private slots: + bool finished() Q_DECL_OVERRIDE; +private: + void send(const QUrl &url); + int _redirects; +}; + } // namespace OCC #endif // NETWORKJOBS_H