Move OAuth dialog to HttpCredentialsGui

This fixes a bug where askFromUser did not display a ui and the client got stuck in reconnecting

Author: TheOneRing

src/gui/accountsettings.cpp

@@ -682,27 +682,19 @@ void AccountSettings::slotAccountStateChanged()
         _model->slotUpdateFolderState(folder);
     }
 
-    auto acceptOAuthLogin = [this]() {
-        if (_askForOAuthLoginDialog != nullptr) {
-            _askForOAuthLoginDialog->accept();
-        }
-    };
-
     const QString server = QStringLiteral("<a href=\"%1\">%1</a>")
                                .arg(Utility::escape(safeUrl.toString()));
 
     switch (state) {
     case AccountState::PausedDueToMetered:
         showConnectionLabel(tr("Sync to %1 is paused due to metered internet connection.").arg(server));
-        acceptOAuthLogin();
         break;
     case AccountState::Connected: {
         QStringList errors;
         if (account->serverSupportLevel() != Account::ServerSupportLevel::Supported) {
             errors << tr("The server version %1 is unsupported! Proceed at your own risk.").arg(account->capabilities().status().versionString());
         }
         showConnectionLabel(tr("Connected to %1.").arg(server), errors);
-        acceptOAuthLogin();
         break;
     }
     case AccountState::ServiceUnavailable:
@@ -715,63 +707,7 @@ void AccountSettings::slotAccountStateChanged()
         showConnectionLabel(tr("Signed out from %1.").arg(server));
         break;
     case AccountState::AskingCredentials: {
-        auto cred = qobject_cast<HttpCredentialsGui *>(account->credentials());
-        if (cred && cred->isUsingOAuth()) {
-            if (_askForOAuthLoginDialog != nullptr) {
-                qCDebug(lcAccountSettings) << "ask for OAuth login dialog is shown already";
-                return;
-            }
-
-            qCDebug(lcAccountSettings) << "showing modal dialog asking user to log in again via OAuth2";
-
-            _askForOAuthLoginDialog = new LoginRequiredDialog(LoginRequiredDialog::Mode::OAuth, ocApp()->gui()->settingsDialog());
-
-            // make sure it's cleaned up since it's not owned by the account settings (also prevents memory leaks)
-            _askForOAuthLoginDialog->setAttribute(Qt::WA_DeleteOnClose);
-
-            _askForOAuthLoginDialog->setTopLabelText(tr("The account %1 is currently logged out.\n\nPlease authenticate using your browser.").arg(account->displayName()));
-
-            auto *contentWidget = qobject_cast<OAuthLoginWidget *>(_askForOAuthLoginDialog->contentWidget());
-
-            connect(cred, &HttpCredentialsGui::authorisationLinkChanged, contentWidget,
-                [cred, contentWidget] { contentWidget->setUrl(cred->authorisationLink()); });
-
-            connect(contentWidget, &OAuthLoginWidget::copyUrlToClipboardButtonClicked, _askForOAuthLoginDialog, [](const QUrl &url) {
-                // TODO: use authorisationLinkAsync
-                qApp->clipboard()->setText(url.toString());
-            });
-
-            connect(contentWidget, &OAuthLoginWidget::openBrowserButtonClicked, cred, &HttpCredentialsGui::openBrowser);
-
-            connect(_askForOAuthLoginDialog, &LoginRequiredDialog::rejected, this, [this]() {
-                // if a user dismisses the dialog, we have no choice but signing them out
-                _accountState->signOutByUi();
-            });
-
-            connect(contentWidget, &OAuthLoginWidget::retryButtonClicked, _askForOAuthLoginDialog, [contentWidget, accountPtr = account]() {
-                auto creds = qobject_cast<HttpCredentialsGui *>(accountPtr->credentials());
-                creds->restartOAuth();
-                contentWidget->hideRetryFrame();
-            });
-
-            connect(cred, &HttpCredentialsGui::oAuthErrorOccurred, _askForOAuthLoginDialog, [loginDialog = _askForOAuthLoginDialog, contentWidget, cred]() {
-                Q_ASSERT(!cred->ready());
-
-                ocApp()->gui()->raiseDialog(loginDialog);
-                contentWidget->showRetryFrame();
-            });
-
-            showConnectionLabel(tr("Reauthorization required."));
-
-            _askForOAuthLoginDialog->open();
-            ocApp()->gui()->raiseDialog(_askForOAuthLoginDialog);
-
-            QTimer::singleShot(0, [contentWidget]() {
-                contentWidget->setFocus(Qt::OtherFocusReason);
-            });
-        } else {
-            showConnectionLabel(tr("Connecting to %1...").arg(server));
-        }
+        showConnectionLabel(tr("Updating credentials for %1...").arg(server));
         break;
     }
     case AccountState::Connecting:

src/gui/accountsettings.h

@@ -103,9 +103,6 @@ protected slots:
     AccountStatePtr _accountState;
     QAction *_toggleSignInOutAction;
     QAction *_toggleReconnect;
-
-    // needed to make sure we show only one dialog at a time
-    QPointer<LoginRequiredDialog> _askForOAuthLoginDialog = nullptr;
 };
 
 } // namespace OCC

src/gui/creds/httpcredentialsgui.cpp

@@ -16,17 +16,17 @@
 #include "creds/httpcredentialsgui.h"
 #include "account.h"
 #include "application.h"
-#include "basicloginwidget.h"
 #include "common/asserts.h"
-#include "loginrequireddialog.h"
+#include "gui/loginrequireddialog/basicloginwidget.h"
+#include "gui/loginrequireddialog/loginrequireddialog.h"
+#include "gui/loginrequireddialog/oauthloginwidget.h"
 #include "networkjobs.h"
 #include "settingsdialog.h"
 #include "theme.h"
 
-#include <QBuffer>
+#include <QClipboard>
 #include <QDesktopServices>
-#include <QMessageBox>
-#include <QNetworkReply>
+#include <QGuiApplication>
 #include <QTimer>
 
 
@@ -42,21 +42,12 @@ void HttpCredentialsGui::openBrowser()
             _asyncAuth->openBrowser();
         } else {
             qCWarning(lcHttpCredentialsGui) << "There is no authentication job running, did the previous attempt fail?";
-            askFromUserAsync();
+            askFromUser();
         }
     }
 }
 
 void HttpCredentialsGui::askFromUser()
-{
-    // This function can be called from AccountState::slotInvalidCredentials,
-    // which (indirectly, through HttpCredentials::invalidateToken) schedules
-    // a cache wipe of the qnam. We can only execute a network job again once
-    // the cache has been cleared, otherwise we'd interfere with the job.
-    QTimer::singleShot(0, this, &HttpCredentialsGui::askFromUserAsync);
-}
-
-void HttpCredentialsGui::askFromUserAsync()
 {
     if (isUsingOAuth()) {
         restartOAuth();
@@ -83,6 +74,11 @@ void HttpCredentialsGui::asyncAuthResult(OAuth::Result r, const QString &token,
     _asyncAuth.reset();
     switch (r) {
     case OAuth::NotSupported:
+        if (_loginRequiredDialog) {
+            _loginRequiredDialog->deleteLater();
+            _loginRequiredDialog.clear();
+        }
+        // show basic auth dialog for historic reasons
         showDialog();
         return;
     case OAuth::ErrorInsecureUrl:
@@ -93,6 +89,9 @@ void HttpCredentialsGui::asyncAuthResult(OAuth::Result r, const QString &token,
         Q_EMIT oAuthErrorOccurred();
         return;
     case OAuth::LoggedIn:
+        if (_loginRequiredDialog) {
+            _loginRequiredDialog->accept();
+        }
         break;
     }
 
@@ -160,10 +159,45 @@ QUrl HttpCredentialsGui::authorisationLink() const
 
 void HttpCredentialsGui::restartOAuth()
 {
+    qCDebug(lcHttpCredentialsGui) << "showing modal dialog asking user to log in again via OAuth2";
+
+    if (!_loginRequiredDialog) {
+        _loginRequiredDialog = new LoginRequiredDialog(LoginRequiredDialog::Mode::OAuth, ocApp()->gui()->settingsDialog());
+
+        // make sure it's cleaned up since it's not owned by the account settings (also prevents memory leaks)
+        _loginRequiredDialog->setAttribute(Qt::WA_DeleteOnClose);
+
+        _loginRequiredDialog->setTopLabelText(
+            tr("The account %1 is currently logged out.\n\nPlease authenticate using your browser.").arg(_account->displayName()));
+
+        auto *contentWidget = qobject_cast<OAuthLoginWidget *>(_loginRequiredDialog->contentWidget());
+        connect(contentWidget, &OAuthLoginWidget::copyUrlToClipboardButtonClicked, _loginRequiredDialog, [](const QUrl &url) {
+            // TODO: use authorisationLinkAsync
+            qApp->clipboard()->setText(url.toString());
+        });
+
+        connect(contentWidget, &OAuthLoginWidget::openBrowserButtonClicked, this, &HttpCredentialsGui::openBrowser);
+        connect(_loginRequiredDialog, &LoginRequiredDialog::rejected, this, &HttpCredentials::requestLogout);
+
+        connect(contentWidget, &OAuthLoginWidget::retryButtonClicked, _loginRequiredDialog, [contentWidget, this]() {
+            restartOAuth();
+            contentWidget->hideRetryFrame();
+        });
+
+        connect(this, &HttpCredentialsGui::oAuthErrorOccurred, _loginRequiredDialog, [contentWidget, this]() {
+            Q_ASSERT(!ready());
+            ocApp()->gui()->raiseDialog(_loginRequiredDialog);
+            contentWidget->showRetryFrame();
+        });
+
+        _loginRequiredDialog->open();
+        ocApp()->gui()->raiseDialog(_loginRequiredDialog);
+    }
+
     _asyncAuth.reset(new AccountBasedOAuth(_account->sharedFromThis(), this));
-    connect(_asyncAuth.data(), &OAuth::result,
-        this, &HttpCredentialsGui::asyncAuthResult);
-    connect(_asyncAuth.data(), &OAuth::authorisationLinkChanged, this, &HttpCredentialsGui::authorisationLinkChanged);
+    connect(_asyncAuth.data(), &OAuth::result, this, &HttpCredentialsGui::asyncAuthResult);
+    connect(_asyncAuth.data(), &OAuth::authorisationLinkChanged, this,
+        [this] { qobject_cast<OAuthLoginWidget *>(_loginRequiredDialog->contentWidget())->setUrl(authorisationLink()); });
     _asyncAuth->startAuthentication();
 }
 

src/gui/creds/httpcredentialsgui.h

@@ -16,10 +16,11 @@
 #pragma once
 #include "creds/httpcredentials.h"
 #include "creds/oauth.h"
+
 #include <QPointer>
-#include <QTcpServer>
 
 namespace OCC {
+class LoginRequiredDialog;
 
 /**
  * @brief The HttpCredentialsGui class
@@ -62,16 +63,13 @@ class HttpCredentialsGui : public HttpCredentials
 private slots:
     void asyncAuthResult(OAuth::Result, const QString &accessToken, const QString &refreshToken);
     void showDialog();
-    void askFromUserAsync();
 
 signals:
-    void authorisationLinkChanged();
-
     void oAuthErrorOccurred();
 
 private:
     QScopedPointer<AccountBasedOAuth, QScopedPointerObjectDeleteLater<AccountBasedOAuth>> _asyncAuth;
-    QPointer<QWidget> _loginRequiredDialog;
+    QPointer<LoginRequiredDialog> _loginRequiredDialog;
 };
 
 } // namespace OCC