Use DetermineAuthTypeJob in HttpCredentials

* Move it to networkjobs
* Minor adjustments to its logic
* Fixes redirect handling for oauth/basic http auth check #6003

diff --git a/src/gui/creds/httpcredentialsgui.cpp b/src/gui/creds/httpcredentialsgui.cpp
index e65554f4987376112896c6e62e4f36fff907b2ec..98e4df907f9a8d618bf26f8f011bd44f56c78452 100644 (file)
--- 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 <QMessageBox>
 #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 3ee02c73a2dec49c063813a2eaf99f345f53caeb..4b2886e39b40440dcf14cea49aeeb2e4f1ba31a0 100644 (file)
--- 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 39f401db6b25c44b45f9c07642ae9d027a7db557..28fa884fae3d1f676aa1b7079fe5cbee36c6d845 100644 (file)
--- 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 a4bf5988e7b5ff1f72ff15cb186371f7e366db8b..6d9ed4c75bbf8ea43b0366bdfa4ca92fb9687086 100644 (file)
--- 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<OwncloudWizard *>(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 271c943a2caa4367207f5abc228bd9679e05cbda..39afeecfceef635785c39d341e53f59ae26fe6a4 100644 (file)
--- 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 accd3998988c3c5fb7e22a09630743a1362980ab..13efa806b12ac2abe3590a1e9bac035b387f2d90 100644 (file)
--- 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 9fb20ced65a87276ed642c8912c7fb99fd1c11b4..a1c24bbd445b82c2120b8acd0f45ee61aa4619d0 100644 (file)
--- 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 78f5bb444f9854501a37546ce3603be1df77de52..e82817a035a5a5bbd0a3c15804b4666d02919bd8 100644 (file)
--- a/src/gui/wizard/owncloudwizard.h
+++ b/src/gui/wizard/owncloudwizard.h
@@ -21,6 +21,7 @@
 #include <QSslKey>
 #include <QSslCertificate>
 
+#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 eaad0070463b198d700cedddf4f5de610ebb2439..c55ed04fdabb0e8ce829cdf7f229a4930c4e7eda 100644 (file)
--- 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 53215de2afd2fea4be255b8745e5b9cb998eb85e..b82588f842c84cdf088a9f5c06fb4247c00899cd 100644 (file)
--- 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 df3bcdabc4fba82234fa2e6201b38a81d09875e2..0dc7ad7328a5801b787b4af36447add309994597 100644 (file)
--- a/src/libsync/creds/httpcredentials.h
+++ b/src/libsync/creds/httpcredentials.h
@@ -19,6 +19,7 @@
 #include <QMap>
 #include <QSslCertificate>
 #include <QSslKey>
+#include <QNetworkRequest>
 #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 613098dda7a80d41233ad8f59ca34d4c54fb14ad..304bb839d6bc4060c6ba6f0125de493a9640e1c2 100644 (file)
--- 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 8929391f2fdb9da97eb1aec28c89027a20e550b5..fb54e95ba0fdc4b34480813cbceddfba3819d788 100644 (file)
--- a/src/libsync/networkjobs.h
+++ b/src/libsync/networkjobs.h
@@ -330,6 +330,32 @@ private:
     QList<QPair<QString, QString>> _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