Restart oauth on error

Fixes: #9196
owncloud · Dec 15, 2021 · 4ba3702 · 4ba3702
1 parent c8dd64d
commit 4ba3702
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 19 deletions.
diff --git a/changelog/unreleased/9196 b/changelog/unreleased/9196
@@ -0,0 +1,6 @@
+Bugfix: Always restart OAuth2 on error
+
+We now always restart the OAuth2 process once we got a result.
+This will ensure that a second try after an error occurred can succeed.
+
+https://github.com/owncloud/client/issues/9196
diff --git a/src/gui/creds/httpcredentialsgui.cpp b/src/gui/creds/httpcredentialsgui.cpp
@@ -92,13 +92,12 @@ void HttpCredentialsGui::askFromUserAsync()
 void HttpCredentialsGui::asyncAuthResult(OAuth::Result r, const QString &user,
     const QString &token, const QString &refreshToken)
 {
+    _asyncAuth.reset();
     switch (r) {
     case OAuth::NotSupported:
         showDialog();
-        _asyncAuth.reset(nullptr);
         return;
     case OAuth::Error:
-        _asyncAuth.reset(nullptr);
         emit asked();
         return;
     case OAuth::LoggedIn:
@@ -111,7 +110,6 @@ void HttpCredentialsGui::asyncAuthResult(OAuth::Result r, const QString &user,
     _refreshToken = refreshToken;
     _ready = true;
     persist();
-    _asyncAuth.reset(nullptr);
     emit asked();
 }
 

diff --git a/src/gui/wizard/owncloudoauthcredspage.cpp b/src/gui/wizard/owncloudoauthcredspage.cpp
@@ -43,18 +43,15 @@ OwncloudOAuthCredsPage::OwncloudOAuthCredsPage()
 
     connect(_ui.openLinkButton, &QCommandLinkButton::clicked, [this] {
         _ui.errorLabel->hide();
-        if (_asyncAuth)
-            _asyncAuth->openBrowser();
+        oauth()->openBrowser();
     });
     _ui.openLinkButton->setContextMenuPolicy(Qt::CustomContextMenu);
     QObject::connect(_ui.openLinkButton, &QWidget::customContextMenuRequested, [this](const QPoint &pos) {
         auto menu = new QMenu(_ui.openLinkButton);
         menu->addAction(tr("Copy link to clipboard"), this, [this] {
-            if (_asyncAuth) {
-                _asyncAuth->authorisationLinkAsync([](const QUrl &link) {
-                    QApplication::clipboard()->setText(link.toString(QUrl::FullyEncoded));
-                });
-            }
+            oauth()->authorisationLinkAsync([](const QUrl &link) {
+                QApplication::clipboard()->setText(link.toString(QUrl::FullyEncoded));
+            });
         });
         menu->setAttribute(Qt::WA_DeleteOnClose);
         menu->popup(_ui.openLinkButton->mapToGlobal(pos));
@@ -66,9 +63,8 @@ void OwncloudOAuthCredsPage::initializePage()
     OwncloudWizard *ocWizard = qobject_cast<OwncloudWizard *>(wizard());
     Q_ASSERT(ocWizard);
     ocWizard->account()->setCredentials(new HttpCredentialsGui);
-    _asyncAuth.reset(new OAuth(ocWizard->account().data(), this));
-    connect(_asyncAuth.data(), &OAuth::result, this, &OwncloudOAuthCredsPage::asyncAuthResult, Qt::QueuedConnection);
-    _asyncAuth->startAuthentication();
+
+    oauth();
     ocApp()->gui()->settingsDialog()->showMinimized();
 }
 
@@ -82,6 +78,7 @@ void OCC::OwncloudOAuthCredsPage::cleanupPage()
 void OwncloudOAuthCredsPage::asyncAuthResult(OAuth::Result r, const QString &user,
     const QString &token, const QString &refreshToken)
 {
+    _asyncAuth.reset();
     switch (r) {
     case OAuth::NotSupported: {
         /* OAuth not supported (can't open browser), fallback to HTTP credentials */
@@ -107,6 +104,16 @@ void OwncloudOAuthCredsPage::asyncAuthResult(OAuth::Result r, const QString &use
     }
 }
 
+OAuth *OwncloudOAuthCredsPage::oauth()
+{
+    if (!_asyncAuth) {
+        _asyncAuth.reset(new OAuth(owncloudWizard()->account().data(), this));
+        connect(_asyncAuth.data(), &OAuth::result, this, &OwncloudOAuthCredsPage::asyncAuthResult, Qt::QueuedConnection);
+        _asyncAuth->startAuthentication();
+    }
+    return _asyncAuth.get();
+}
+
 int OwncloudOAuthCredsPage::nextId() const
 {
     if (Theme::instance()->wizardSkipAdvancedPage()) {

diff --git a/src/gui/wizard/owncloudoauthcredspage.h b/src/gui/wizard/owncloudoauthcredspage.h
@@ -28,6 +28,7 @@
 
 
 namespace OCC {
+class OAuth;
 
 
 class OwncloudOAuthCredsPage : public AbstractCredentialsWizardPage
@@ -51,12 +52,13 @@ public Q_SLOTS:
 signals:
     void connectToOCUrl(const QString &);
 
-public:
+private:
+    OAuth *oauth();
     QString _user;
     QString _token;
     QString _refreshToken;
-    QScopedPointer<OAuth> _asyncAuth;
     Ui_OwncloudOAuthCredsPage _ui;
+    QScopedPointer<OAuth> _asyncAuth;
 };
 
 } // namespace OCC
diff --git a/src/libsync/creds/oauth.cpp b/src/libsync/creds/oauth.cpp
@@ -224,17 +224,17 @@ void OAuth::startAuthentication()
                     httpReplyAndClose(socket, QByteArrayLiteral("400 Bad Request"), QByteArrayLiteral("<html><head><title>400 Bad Request</title></head><body><center><h1>400 Bad Request</h1></center></body></html>"));
                     return;
                 }
+                // we only allow one response
+                qCDebug(lcOauth) << "Recieved the first valid request, stoping to listen";
+                _server.close();
+
                 auto job = postTokenRequest({
                     { QStringLiteral("grant_type"), QStringLiteral("authorization_code") },
                     { QStringLiteral("code"), args.queryItemValue(QStringLiteral("code")) },
                     { QStringLiteral("redirect_uri"), QStringLiteral("%1:%2").arg(_redirectUrl, QString::number(_server.serverPort())) },
                     { QStringLiteral("code_verifier"), QString::fromUtf8(_pkceCodeVerifier) },
                 });
                 QObject::connect(job, &SimpleNetworkJob::finishedSignal, this, [this, socket](QNetworkReply *reply) {
-                    qScopeGuard([this] {
-                        // close the server once we are done here
-                        _server.close();
-                    });
                     const auto jsonData = reply->readAll();
                     QJsonParseError jsonParseError;
                     const auto data = QJsonDocument::fromJson(jsonData, &jsonParseError).object().toVariantMap();