From 4ba37025f4b5ac4a2a6cde6aac7d4a5896cfbac1 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Fri, 10 Dec 2021 10:52:38 +0100 Subject: [PATCH] Restart oauth on error Fixes: #9196 --- changelog/unreleased/9196 | 6 +++++ src/gui/creds/httpcredentialsgui.cpp | 4 +--- src/gui/wizard/owncloudoauthcredspage.cpp | 27 ++++++++++++++--------- src/gui/wizard/owncloudoauthcredspage.h | 6 +++-- src/libsync/creds/oauth.cpp | 8 +++---- 5 files changed, 32 insertions(+), 19 deletions(-) create mode 100644 changelog/unreleased/9196 diff --git a/changelog/unreleased/9196 b/changelog/unreleased/9196 new file mode 100644 index 00000000000..18503f1df1e --- /dev/null +++ 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 index c2743d5818e..e035f1a0931 100644 --- 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 index e577c4faca5..93865ce6c11 100644 --- 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(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 index 32341eb1cde..3cf7b93f156 100644 --- 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 _asyncAuth; Ui_OwncloudOAuthCredsPage _ui; + QScopedPointer _asyncAuth; }; } // namespace OCC diff --git a/src/libsync/creds/oauth.cpp b/src/libsync/creds/oauth.cpp index e258bca7711..e729d63dccb 100644 --- a/src/libsync/creds/oauth.cpp +++ b/src/libsync/creds/oauth.cpp @@ -224,6 +224,10 @@ void OAuth::startAuthentication() httpReplyAndClose(socket, QByteArrayLiteral("400 Bad Request"), QByteArrayLiteral("400 Bad Request

400 Bad Request

")); 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")) }, @@ -231,10 +235,6 @@ void OAuth::startAuthentication() { 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();