From e097ba937d085d8f626feba46161028d0a0d49e6 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Tue, 29 Aug 2023 14:27:50 +0800 Subject: [PATCH] Remove OAuth authentication method as it is now unused Signed-off-by: Claudio Cambra --- src/gui/CMakeLists.txt | 5 - src/gui/accountsettings.cpp | 14 +- src/gui/accountstate.cpp | 4 - src/gui/creds/httpcredentialsgui.cpp | 38 +-- src/gui/creds/httpcredentialsgui.h | 25 -- src/gui/creds/oauth.cpp | 187 ------------ src/gui/creds/oauth.h | 76 ----- src/gui/creds/webflowcredentials.cpp | 2 +- src/gui/guiutility.cpp | 1 - src/gui/owncloudsetupwizard.cpp | 2 +- src/gui/wizard/owncloudoauthcredspage.cpp | 140 --------- src/gui/wizard/owncloudoauthcredspage.h | 66 ----- src/gui/wizard/owncloudoauthcredspage.ui | 100 ------- src/gui/wizard/owncloudsetuppage.cpp | 2 - src/gui/wizard/owncloudwizard.cpp | 16 +- src/gui/wizard/owncloudwizard.h | 2 - src/gui/wizard/owncloudwizardcommon.h | 1 - src/libsync/creds/httpcredentials.cpp | 119 +------- src/libsync/creds/httpcredentials.h | 21 +- src/libsync/networkjobs.cpp | 17 +- src/libsync/networkjobs.h | 1 - src/libsync/propagatedownload.cpp | 2 +- src/libsync/theme.cpp | 10 - src/libsync/theme.h | 7 - test/CMakeLists.txt | 2 - test/testoauth.cpp | 337 ---------------------- 26 files changed, 30 insertions(+), 1167 deletions(-) delete mode 100644 src/gui/creds/oauth.cpp delete mode 100644 src/gui/creds/oauth.h delete mode 100644 src/gui/wizard/owncloudoauthcredspage.cpp delete mode 100644 src/gui/wizard/owncloudoauthcredspage.h delete mode 100644 src/gui/wizard/owncloudoauthcredspage.ui delete mode 100644 test/testoauth.cpp diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt index e4af0e33a6fef..9aacc81f13bc6 100644 --- a/src/gui/CMakeLists.txt +++ b/src/gui/CMakeLists.txt @@ -49,7 +49,6 @@ set(client_UI_SRCS wizard/owncloudadvancedsetuppage.ui wizard/owncloudconnectionmethoddialog.ui wizard/owncloudhttpcredspage.ui - wizard/owncloudoauthcredspage.ui wizard/owncloudsetupnocredspage.ui wizard/webview.ui wizard/welcomepage.ui @@ -223,8 +222,6 @@ set(client_SRCS creds/credentialsfactory.cpp creds/httpcredentialsgui.h creds/httpcredentialsgui.cpp - creds/oauth.h - creds/oauth.cpp creds/flow2auth.h creds/flow2auth.cpp creds/webflowcredentials.h @@ -241,8 +238,6 @@ set(client_SRCS wizard/owncloudconnectionmethoddialog.cpp wizard/owncloudhttpcredspage.h wizard/owncloudhttpcredspage.cpp - wizard/owncloudoauthcredspage.h - wizard/owncloudoauthcredspage.cpp wizard/flow2authcredspage.h wizard/flow2authcredspage.cpp wizard/flow2authwidget.h diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index 994fbe3c327ed..312e1fdf02e20 100644 --- a/src/gui/accountsettings.cpp +++ b/src/gui/accountsettings.cpp @@ -1271,19 +1271,7 @@ void AccountSettings::slotAccountStateChanged() showConnectionLabel(tr("Signed out from %1.").arg(serverWithUser)); break; case AccountState::AskingCredentials: { - QUrl url; - if (const auto cred = qobject_cast(account->credentials())) { - connect(cred, &HttpCredentialsGui::authorisationLinkChanged, - this, &AccountSettings::slotAccountStateChanged, Qt::UniqueConnection); - url = cred->authorisationLink(); - } - if (url.isValid()) { - showConnectionLabel(tr("Obtaining authorization from the browser. " - "Click here to re-open the browser.") - .arg(url.toString(QUrl::FullyEncoded))); - } else { - showConnectionLabel(tr("Connecting to %1 …").arg(serverWithUser)); - } + showConnectionLabel(tr("Connecting to %1 …").arg(serverWithUser)); break; } case AccountState::NetworkError: diff --git a/src/gui/accountstate.cpp b/src/gui/accountstate.cpp index 3133e22cd7590..56ae51489b8a2 100644 --- a/src/gui/accountstate.cpp +++ b/src/gui/accountstate.cpp @@ -456,10 +456,6 @@ void AccountState::handleInvalidCredentials() if (account()->credentials()->ready()) { account()->credentials()->invalidateToken(); } - if (auto creds = qobject_cast(account()->credentials())) { - if (creds->refreshAccessToken()) - return; - } account()->credentials()->askFromUser(); } diff --git a/src/gui/creds/httpcredentialsgui.cpp b/src/gui/creds/httpcredentialsgui.cpp index 915a6c224fb77..4e1d8ef1e630e 100644 --- a/src/gui/creds/httpcredentialsgui.cpp +++ b/src/gui/creds/httpcredentialsgui.cpp @@ -46,16 +46,7 @@ void HttpCredentialsGui::askFromUserAsync() // First, we will check what kind of auth we need. auto job = new DetermineAuthTypeJob(_account->sharedFromThis(), this); QObject::connect(job, &DetermineAuthTypeJob::authType, this, [this](DetermineAuthTypeJob::AuthType type) { - if (type == DetermineAuthTypeJob::OAuth) { - _asyncAuth.reset(new OAuth(_account, this)); - _asyncAuth->_expectedUser = _account->davUser(); - connect(_asyncAuth.data(), &OAuth::result, - this, &HttpCredentialsGui::asyncAuthResult); - connect(_asyncAuth.data(), &OAuth::destroyed, - this, &HttpCredentialsGui::authorisationLinkChanged); - _asyncAuth->start(); - emit authorisationLinkChanged(); - } else if (type == DetermineAuthTypeJob::Basic) { + if (type == DetermineAuthTypeJob::Basic) { showDialog(); } else { // Shibboleth? @@ -66,32 +57,6 @@ void HttpCredentialsGui::askFromUserAsync() job->start(); } -void HttpCredentialsGui::asyncAuthResult(OAuth::Result r, const QString &user, - const QString &token, const QString &refreshToken) -{ - switch (r) { - case OAuth::NotSupported: - showDialog(); - _asyncAuth.reset(nullptr); - return; - case OAuth::Error: - _asyncAuth.reset(nullptr); - emit asked(); - return; - case OAuth::LoggedIn: - break; - } - - ASSERT(_user == user); // ensured by _asyncAuth - - _password = token; - _refreshToken = refreshToken; - _ready = true; - persist(); - _asyncAuth.reset(nullptr); - emit asked(); -} - void HttpCredentialsGui::showDialog() { QString msg = tr("Please enter %1 password:
" @@ -128,7 +93,6 @@ void HttpCredentialsGui::showDialog() connect(dialog, &QDialog::finished, this, [this, dialog](int result) { if (result == QDialog::Accepted) { _password = dialog->textValue(); - _refreshToken.clear(); _ready = true; persist(); } diff --git a/src/gui/creds/httpcredentialsgui.h b/src/gui/creds/httpcredentialsgui.h index df059a0a7b3ac..2e220a533e57f 100644 --- a/src/gui/creds/httpcredentialsgui.h +++ b/src/gui/creds/httpcredentialsgui.h @@ -15,7 +15,6 @@ #pragma once #include "creds/httpcredentials.h" -#include "creds/oauth.h" #include #include @@ -38,37 +37,13 @@ class HttpCredentialsGui : public HttpCredentials : HttpCredentials(user, password, clientCertBundle, clientCertPassword) { } - HttpCredentialsGui(const QString &user, const QString &password, const QString &refreshToken, - const QByteArray &clientCertBundle, const QByteArray &clientCertPassword) - : HttpCredentials(user, password, clientCertBundle, clientCertPassword) - { - _refreshToken = refreshToken; - } - /** - * This will query the server and either uses OAuth via _asyncAuth->start() - * or call showDialog to ask the password - */ void askFromUser() override; - /** - * In case of oauth, return an URL to the link to open the browser. - * An invalid URL otherwise - */ - [[nodiscard]] QUrl authorisationLink() const { return _asyncAuth ? _asyncAuth->authorisationLink() : QUrl(); } - static QString requestAppPasswordText(const Account *account); private slots: - void asyncAuthResult(OAuth::Result, const QString &user, const QString &accessToken, const QString &refreshToken); void showDialog(); void askFromUserAsync(); - -signals: - void authorisationLinkChanged(); - -private: - - QScopedPointer> _asyncAuth; }; } // namespace OCC diff --git a/src/gui/creds/oauth.cpp b/src/gui/creds/oauth.cpp deleted file mode 100644 index eaa25b97ae4ff..0000000000000 --- a/src/gui/creds/oauth.cpp +++ /dev/null @@ -1,187 +0,0 @@ -/* - * Copyright (C) by Olivier Goffart - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY - * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * for more details. - */ - -#include -#include -#include -#include -#include "account.h" -#include "creds/oauth.h" -#include -#include -#include "theme.h" -#include "networkjobs.h" -#include "creds/httpcredentials.h" -#include "guiutility.h" - -namespace OCC { - -Q_LOGGING_CATEGORY(lcOauth, "nextcloud.sync.credentials.oauth", QtInfoMsg) - -OAuth::~OAuth() = default; - -static void httpReplyAndClose(QTcpSocket *socket, const char *code, const char *html, - const char *moreHeaders = nullptr) -{ - if (!socket) - return; // socket can have been deleted if the browser was closed - socket->write("HTTP/1.1 "); - socket->write(code); - socket->write("\r\nContent-Type: text/html\r\nConnection: close\r\nContent-Length: "); - socket->write(QByteArray::number(qstrlen(html))); - if (moreHeaders) { - socket->write("\r\n"); - socket->write(moreHeaders); - } - socket->write("\r\n\r\n"); - socket->write(html); - socket->disconnectFromHost(); - // We don't want that deleting the server too early prevent queued data to be sent on this socket. - // The socket will be deleted after disconnection because disconnected is connected to deleteLater - socket->setParent(nullptr); -} - -void OAuth::start() -{ - // Listen on the socket to get a port which will be used in the redirect_uri - if (!_server.listen(QHostAddress::LocalHost)) { - emit result(NotSupported, QString()); - return; - } - - if (!openBrowser()) - return; - - QObject::connect(&_server, &QTcpServer::newConnection, this, [this] { - while (QPointer socket = _server.nextPendingConnection()) { - QObject::connect(socket.data(), &QTcpSocket::disconnected, socket.data(), &QTcpSocket::deleteLater); - QObject::connect(socket.data(), &QIODevice::readyRead, this, [this, socket] { - QByteArray peek = socket->peek(qMin(socket->bytesAvailable(), 4000LL)); //The code should always be within the first 4K - if (peek.indexOf('\n') < 0) - return; // wait until we find a \n - static const QRegularExpression rx("^GET /\\?code=([a-zA-Z0-9]+)[& ]"); // Match a /?code=... URL - const auto rxMatch = rx.match(peek); - if (!rxMatch.hasMatch()) { - httpReplyAndClose(socket, "404 Not Found", "404 Not Found

404 Not Found

"); - return; - } - - QString code = rxMatch.captured(1); // The 'code' is the first capture of the regexp - - QUrl requestToken = Utility::concatUrlPath(_account->url().toString(), QLatin1String("/index.php/apps/oauth2/api/v1/token")); - QNetworkRequest req; - req.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded"); - - QString basicAuth = QString("%1:%2").arg( - Theme::instance()->oauthClientId(), Theme::instance()->oauthClientSecret()); - req.setRawHeader("Authorization", "Basic " + basicAuth.toUtf8().toBase64()); - // We just added the Authorization header, don't let HttpCredentialsAccessManager tamper with it - req.setAttribute(HttpCredentials::DontAddCredentialsAttribute, true); - - auto requestBody = new QBuffer; - QUrlQuery arguments(QString( - "grant_type=authorization_code&code=%1&redirect_uri=http://localhost:%2") - .arg(code, QString::number(_server.serverPort()))); - requestBody->setData(arguments.query(QUrl::FullyEncoded).toLatin1()); - - auto job = _account->sendRequest("POST", requestToken, req, requestBody); - job->setTimeout(qMin(30 * 1000ll, job->timeoutMsec())); - QObject::connect(job, &SimpleNetworkJob::finishedSignal, this, [this, socket](QNetworkReply *reply) { - auto jsonData = reply->readAll(); - QJsonParseError jsonParseError{}; - QJsonObject json = QJsonDocument::fromJson(jsonData, &jsonParseError).object(); - QString accessToken = json["access_token"].toString(); - QString refreshToken = json["refresh_token"].toString(); - QString user = json["user_id"].toString(); - QUrl messageUrl = json["message_url"].toString(); - - if (reply->error() != QNetworkReply::NoError || jsonParseError.error != QJsonParseError::NoError - || jsonData.isEmpty() || json.isEmpty() || refreshToken.isEmpty() || accessToken.isEmpty() - || json["token_type"].toString() != QLatin1String("Bearer")) { - QString errorReason; - QString errorFromJson = json["error"].toString(); - if (!errorFromJson.isEmpty()) { - errorReason = tr("Error returned from the server: %1") - .arg(errorFromJson.toHtmlEscaped()); - } else if (reply->error() != QNetworkReply::NoError) { - errorReason = tr("There was an error accessing the \"token\" endpoint:
%1") - .arg(reply->errorString().toHtmlEscaped()); - } else if (jsonData.isEmpty()) { - // Can happen if a funky load balancer strips away POST data, e.g. BigIP APM my.policy - errorReason = tr("Empty JSON from OAuth2 redirect"); - // We explicitly have this as error case since the json qcWarning output below is misleading, - // it will show a fake json will null values that actually never was received like this as - // soon as you access json["whatever"] the debug output json will claim to have "whatever":null - } else if (jsonParseError.error != QJsonParseError::NoError) { - errorReason = tr("Could not parse the JSON returned from the server:
%1") - .arg(jsonParseError.errorString()); - } else { - errorReason = tr("The reply from the server did not contain all expected fields"); - } - qCWarning(lcOauth) << "Error when getting the accessToken" << json << errorReason; - httpReplyAndClose(socket, "500 Internal Server Error", - tr("

Login Error

%1

").arg(errorReason).toUtf8().constData()); - emit result(Error); - return; - } - if (!_expectedUser.isNull() && user != _expectedUser) { - // Connected with the wrong user - QString message = tr("

Wrong account

" - "

You logged in with the account %1, but must log in with the account %2.
" - "Please log out of %3 in another tab, then click here " - "and log in with %2.

") - .arg(user, _expectedUser, Theme::instance()->appNameGUI(), - authorisationLink().toString(QUrl::FullyEncoded)); - httpReplyAndClose(socket, "200 OK", message.toUtf8().constData()); - // We are still listening on the socket so we will get the new connection - return; - } - const char *loginSuccessfullHtml = "

Login Successful

You can close this window.

"; - if (messageUrl.isValid()) { - httpReplyAndClose(socket, "303 See Other", loginSuccessfullHtml, - QByteArray("Location: " + messageUrl.toEncoded()).constData()); - } else { - httpReplyAndClose(socket, "200 OK", loginSuccessfullHtml); - } - emit result(LoggedIn, user, accessToken, refreshToken); - }); - }); - } - }); -} - -QUrl OAuth::authorisationLink() const -{ - Q_ASSERT(_server.isListening()); - QUrlQuery query; - query.setQueryItems({ { QLatin1String("response_type"), QLatin1String("code") }, - { QLatin1String("client_id"), Theme::instance()->oauthClientId() }, - { QLatin1String("redirect_uri"), QLatin1String("http://localhost:") + QString::number(_server.serverPort()) } }); - if (!_expectedUser.isNull()) - query.addQueryItem("user", _expectedUser); - QUrl url = Utility::concatUrlPath(_account->url(), QLatin1String("/index.php/apps/oauth2/authorize"), query); - return url; -} - -bool OAuth::openBrowser() -{ - if (!Utility::openBrowser(authorisationLink())) { - // We cannot open the browser, then we claim we don't support OAuth. - emit result(NotSupported, QString()); - return false; - } - return true; -} - -} // namespace OCC diff --git a/src/gui/creds/oauth.h b/src/gui/creds/oauth.h deleted file mode 100644 index c07cdbc8697ee..0000000000000 --- a/src/gui/creds/oauth.h +++ /dev/null @@ -1,76 +0,0 @@ -/* - * Copyright (C) by Olivier Goffart - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY - * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * for more details. - */ - -#pragma once -#include -#include -#include -#include "accountfwd.h" - -namespace OCC { - -/** - * Job that do the authorization grant and fetch the access token - * - * Normal workflow: - * - * --> start() - * | - * +----> openBrowser() open the browser to the login page, redirects to http://localhost:xxx - * | - * +----> _server starts listening on a TCP port waiting for an HTTP request with a 'code' - * | - * v - * request the access_token and the refresh_token via 'apps/oauth2/api/v1/token' - * | - * v - * emit result(...) - * - */ -class OAuth : public QObject -{ - Q_OBJECT -public: - OAuth(Account *account, QObject *parent) - : QObject(parent) - , _account(account) - { - } - ~OAuth() override; - - enum Result { NotSupported, - LoggedIn, - Error }; - Q_ENUM(Result); - void start(); - bool openBrowser(); - [[nodiscard]] QUrl authorisationLink() const; - -signals: - /** - * The state has changed. - * when logged in, token has the value of the token. - */ - void result(OAuth::Result result, const QString &user = QString(), const QString &token = QString(), const QString &refreshToken = QString()); - -private: - Account *_account; - QTcpServer _server; - -public: - QString _expectedUser; -}; - - -} // namespace OCC diff --git a/src/gui/creds/webflowcredentials.cpp b/src/gui/creds/webflowcredentials.cpp index 9958d1b7d55a7..04d9fc7276e14 100644 --- a/src/gui/creds/webflowcredentials.cpp +++ b/src/gui/creds/webflowcredentials.cpp @@ -146,7 +146,7 @@ void WebFlowCredentials::askFromUser() { // Do a DetermineAuthTypeJob to make sure that the server is still using Flow2 auto job = new DetermineAuthTypeJob(_account->sharedFromThis(), this); connect(job, &DetermineAuthTypeJob::authType, [this](DetermineAuthTypeJob::AuthType type) { - // LoginFlowV2 > WebViewFlow > OAuth > Shib > Basic + // LoginFlowV2 > WebViewFlow > Shib > Basic #ifdef WITH_WEBENGINE bool useFlow2 = (type != DetermineAuthTypeJob::WebViewFlow); #else // WITH_WEBENGINE diff --git a/src/gui/guiutility.cpp b/src/gui/guiutility.cpp index 9a86075b805bf..6e5fe203e5ef8 100644 --- a/src/gui/guiutility.cpp +++ b/src/gui/guiutility.cpp @@ -31,7 +31,6 @@ bool Utility::openBrowser(const QUrl &url, QWidget *errorWidgetParent) const QStringList allowedUrlSchemes = { "http", "https", - "oauthtest" }; if (!allowedUrlSchemes.contains(url.scheme())) { diff --git a/src/gui/owncloudsetupwizard.cpp b/src/gui/owncloudsetupwizard.cpp index b0354b58e03b0..5b4e6e54ac9b3 100644 --- a/src/gui/owncloudsetupwizard.cpp +++ b/src/gui/owncloudsetupwizard.cpp @@ -430,7 +430,7 @@ void OwncloudSetupWizard::slotAuthError() // bring wizard to top _ocWizard->bringToTop(); - if (_ocWizard->currentId() == WizardCommon::Page_OAuthCreds || _ocWizard->currentId() == WizardCommon::Page_Flow2AuthCreds) { + if (_ocWizard->currentId() == WizardCommon::Page_Flow2AuthCreds) { _ocWizard->back(); } _ocWizard->displayError(errorMsg, _ocWizard->currentId() == WizardCommon::Page_ServerSetup && checkDowngradeAdvised(reply)); diff --git a/src/gui/wizard/owncloudoauthcredspage.cpp b/src/gui/wizard/owncloudoauthcredspage.cpp deleted file mode 100644 index 267830dc1c803..0000000000000 --- a/src/gui/wizard/owncloudoauthcredspage.cpp +++ /dev/null @@ -1,140 +0,0 @@ -/* - * Copyright (C) by Olivier Goffart - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY - * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * for more details. - */ - -#include -#include -#include - -#include "wizard/owncloudoauthcredspage.h" -#include "theme.h" -#include "account.h" -#include "cookiejar.h" -#include "wizard/owncloudwizardcommon.h" -#include "wizard/owncloudwizard.h" -#include "creds/httpcredentialsgui.h" -#include "creds/credentialsfactory.h" - -namespace OCC { - -OwncloudOAuthCredsPage::OwncloudOAuthCredsPage() - : AbstractCredentialsWizardPage() -{ - _ui.setupUi(this); - - Theme *theme = Theme::instance(); - _ui.topLabel->hide(); - _ui.bottomLabel->hide(); - QVariant variant = theme->customMedia(Theme::oCSetupTop); - WizardCommon::setupCustomMedia(variant, _ui.topLabel); - variant = theme->customMedia(Theme::oCSetupBottom); - WizardCommon::setupCustomMedia(variant, _ui.bottomLabel); - - WizardCommon::initErrorLabel(_ui.errorLabel); - - setTitle(WizardCommon::titleTemplate().arg(tr("Connect to %1").arg(Theme::instance()->appNameGUI()))); - setSubTitle(WizardCommon::subTitleTemplate().arg(tr("Login in your browser"))); - - connect(_ui.openLinkButton, &QCommandLinkButton::clicked, this, &OwncloudOAuthCredsPage::slotOpenBrowser); - connect(_ui.copyLinkButton, &QCommandLinkButton::clicked, this, &OwncloudOAuthCredsPage::slotCopyLinkToClipboard); -} - -void OwncloudOAuthCredsPage::initializePage() -{ - auto *ocWizard = qobject_cast(wizard()); - Q_ASSERT(ocWizard); - ocWizard->account()->setCredentials(CredentialsFactory::create("http")); - _asyncAuth.reset(new OAuth(ocWizard->account().data(), this)); - connect(_asyncAuth.data(), &OAuth::result, this, &OwncloudOAuthCredsPage::asyncAuthResult, Qt::QueuedConnection); - _asyncAuth->start(); - - // Don't hide the wizard (avoid user confusion)! - //wizard()->hide(); -} - -void OCC::OwncloudOAuthCredsPage::cleanupPage() -{ - // The next or back button was activated, show the wizard again - wizard()->show(); - _asyncAuth.reset(); -} - -void OwncloudOAuthCredsPage::asyncAuthResult(OAuth::Result r, const QString &user, - const QString &token, const QString &refreshToken) -{ - switch (r) { - case OAuth::NotSupported: { - /* OAuth not supported (can't open browser), fallback to HTTP credentials */ - auto *ocWizard = qobject_cast(wizard()); - ocWizard->back(); - ocWizard->setAuthType(DetermineAuthTypeJob::Basic); - break; - } - case OAuth::Error: - /* Error while getting the access token. (Timeout, or the server did not accept our client credentials */ - _ui.errorLabel->show(); - wizard()->show(); - break; - case OAuth::LoggedIn: { - _token = token; - _user = user; - _refreshToken = refreshToken; - auto *ocWizard = qobject_cast(wizard()); - Q_ASSERT(ocWizard); - emit connectToOCUrl(ocWizard->account()->url().toString()); - break; - } - } -} - -int OwncloudOAuthCredsPage::nextId() const -{ - return WizardCommon::Page_AdvancedSetup; -} - -void OwncloudOAuthCredsPage::setConnected() -{ - wizard()->show(); -} - -AbstractCredentials *OwncloudOAuthCredsPage::getCredentials() const -{ - auto *ocWizard = qobject_cast(wizard()); - Q_ASSERT(ocWizard); - return new HttpCredentialsGui(_user, _token, _refreshToken, - ocWizard->_clientCertBundle, ocWizard->_clientCertPassword); -} - -bool OwncloudOAuthCredsPage::isComplete() const -{ - return false; /* We can never go forward manually */ -} - -void OwncloudOAuthCredsPage::slotOpenBrowser() -{ - if (_ui.errorLabel) - _ui.errorLabel->hide(); - - qobject_cast(wizard())->account()->clearCookieJar(); // #6574 - - if (_asyncAuth) - _asyncAuth->openBrowser(); -} - -void OwncloudOAuthCredsPage::slotCopyLinkToClipboard() -{ - if (_asyncAuth) - QApplication::clipboard()->setText(_asyncAuth->authorisationLink().toString(QUrl::FullyEncoded)); -} - -} // namespace OCC diff --git a/src/gui/wizard/owncloudoauthcredspage.h b/src/gui/wizard/owncloudoauthcredspage.h deleted file mode 100644 index 2702aa3300cdb..0000000000000 --- a/src/gui/wizard/owncloudoauthcredspage.h +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright (C) by Olivier Goffart - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY - * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * for more details. - */ - -#pragma once - -#include -#include -#include -#include -#include - -#include "wizard/abstractcredswizardpage.h" -#include "accountfwd.h" -#include "creds/oauth.h" - -#include "ui_owncloudoauthcredspage.h" - - -namespace OCC { - - -class OwncloudOAuthCredsPage : public AbstractCredentialsWizardPage -{ - Q_OBJECT -public: - OwncloudOAuthCredsPage(); - - [[nodiscard]] AbstractCredentials *getCredentials() const override; - - void initializePage() override; - void cleanupPage() override; - [[nodiscard]] int nextId() const override; - void setConnected(); - [[nodiscard]] bool isComplete() const override; - -public Q_SLOTS: - void asyncAuthResult(OAuth::Result, const QString &user, const QString &token, - const QString &reniewToken); - -signals: - void connectToOCUrl(const QString &); - -public: - QString _user; - QString _token; - QString _refreshToken; - QScopedPointer _asyncAuth; - Ui_OwncloudOAuthCredsPage _ui{}; - -protected slots: - void slotOpenBrowser(); - void slotCopyLinkToClipboard(); -}; - -} // namespace OCC diff --git a/src/gui/wizard/owncloudoauthcredspage.ui b/src/gui/wizard/owncloudoauthcredspage.ui deleted file mode 100644 index 03682cae10d40..0000000000000 --- a/src/gui/wizard/owncloudoauthcredspage.ui +++ /dev/null @@ -1,100 +0,0 @@ - - - OwncloudOAuthCredsPage - - - - 0 - 0 - 424 - 373 - - - - Form - - - - - - TextLabel - - - Qt::RichText - - - Qt::AlignCenter - - - true - - - - - - - Please switch to your browser to proceed. - - - true - - - - - - - An error occurred while connecting. Please try again. - - - Qt::PlainText - - - - - - - Re-open Browser - - - - - - - - 50 - false - - - - Copy link - - - - - - - Qt::Vertical - - - - 20 - 127 - - - - - - - - TextLabel - - - Qt::RichText - - - - - - - - diff --git a/src/gui/wizard/owncloudsetuppage.cpp b/src/gui/wizard/owncloudsetuppage.cpp index 69656ea66b701..c7134478ab0ae 100644 --- a/src/gui/wizard/owncloudsetuppage.cpp +++ b/src/gui/wizard/owncloudsetuppage.cpp @@ -209,8 +209,6 @@ int OwncloudSetupPage::nextId() const switch (_authType) { case DetermineAuthTypeJob::Basic: return WizardCommon::Page_HttpCreds; - case DetermineAuthTypeJob::OAuth: - return WizardCommon::Page_OAuthCreds; case DetermineAuthTypeJob::LoginFlowV2: return WizardCommon::Page_Flow2AuthCreds; #ifdef WITH_WEBENGINE diff --git a/src/gui/wizard/owncloudwizard.cpp b/src/gui/wizard/owncloudwizard.cpp index afc19c40986f0..faac2f5ad8934 100644 --- a/src/gui/wizard/owncloudwizard.cpp +++ b/src/gui/wizard/owncloudwizard.cpp @@ -23,7 +23,6 @@ #include "wizard/welcomepage.h" #include "wizard/owncloudsetuppage.h" #include "wizard/owncloudhttpcredspage.h" -#include "wizard/owncloudoauthcredspage.h" #include "wizard/owncloudadvancedsetuppage.h" #include "wizard/webviewpage.h" #include "wizard/flow2authcredspage.h" @@ -49,7 +48,6 @@ OwncloudWizard::OwncloudWizard(QWidget *parent) , _welcomePage(new WelcomePage(this)) , _setupPage(new OwncloudSetupPage(this)) , _httpCredsPage(new OwncloudHttpCredsPage(this)) - , _browserCredsPage(new OwncloudOAuthCredsPage) , _flow2CredsPage(new Flow2AuthCredsPage) , _advancedSetupPage(new OwncloudAdvancedSetupPage(this)) #ifdef WITH_WEBENGINE @@ -64,7 +62,6 @@ OwncloudWizard::OwncloudWizard(QWidget *parent) setPage(WizardCommon::Page_Welcome, _welcomePage); setPage(WizardCommon::Page_ServerSetup, _setupPage); setPage(WizardCommon::Page_HttpCreds, _httpCredsPage); - setPage(WizardCommon::Page_OAuthCreds, _browserCredsPage); setPage(WizardCommon::Page_Flow2AuthCreds, _flow2CredsPage); setPage(WizardCommon::Page_AdvancedSetup, _advancedSetupPage); #ifdef WITH_WEBENGINE @@ -79,7 +76,6 @@ OwncloudWizard::OwncloudWizard(QWidget *parent) connect(this, &QWizard::currentIdChanged, this, &OwncloudWizard::slotCurrentPageChanged); connect(_setupPage, &OwncloudSetupPage::determineAuthType, this, &OwncloudWizard::determineAuthType); connect(_httpCredsPage, &OwncloudHttpCredsPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); - connect(_browserCredsPage, &OwncloudOAuthCredsPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); connect(_flow2CredsPage, &Flow2AuthCredsPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); #ifdef WITH_WEBENGINE connect(_webViewPage, &WebViewPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); @@ -233,10 +229,6 @@ void OwncloudWizard::successfulStep() _httpCredsPage->setConnected(); break; - case WizardCommon::Page_OAuthCreds: - _browserCredsPage->setConnected(); - break; - case WizardCommon::Page_Flow2AuthCreds: _flow2CredsPage->setConnected(); break; @@ -280,9 +272,7 @@ void OwncloudWizard::setAuthType(DetermineAuthTypeJob::AuthType type) { _setupPage->setAuthType(type); - if (type == DetermineAuthTypeJob::OAuth) { - _credentialsPage = _browserCredsPage; - } else if (type == DetermineAuthTypeJob::LoginFlowV2) { + if (type == DetermineAuthTypeJob::LoginFlowV2) { _credentialsPage = _flow2CredsPage; #ifdef WITH_WEBENGINE } else if (type == DetermineAuthTypeJob::WebViewFlow) { @@ -329,8 +319,8 @@ void OwncloudWizard::slotCurrentPageChanged(int id) emit clearPendingRequests(); } - if (id == WizardCommon::Page_AdvancedSetup && (_credentialsPage == _browserCredsPage || _credentialsPage == _flow2CredsPage)) { - // For OAuth, disable the back button in the Page_AdvancedSetup because we don't want + if (id == WizardCommon::Page_AdvancedSetup && _credentialsPage == _flow2CredsPage) { + // Disable the back button in the Page_AdvancedSetup because we don't want // to re-open the browser. button(QWizard::BackButton)->setEnabled(false); } diff --git a/src/gui/wizard/owncloudwizard.h b/src/gui/wizard/owncloudwizard.h index 2ebe0f08baea3..58d3e915d9d1b 100644 --- a/src/gui/wizard/owncloudwizard.h +++ b/src/gui/wizard/owncloudwizard.h @@ -32,7 +32,6 @@ Q_DECLARE_LOGGING_CATEGORY(lcWizard) class WelcomePage; class OwncloudSetupPage; class OwncloudHttpCredsPage; -class OwncloudOAuthCredsPage; class OwncloudAdvancedSetupPage; class OwncloudWizardResultPage; class AbstractCredentials; @@ -122,7 +121,6 @@ public slots: WelcomePage *_welcomePage; OwncloudSetupPage *_setupPage; OwncloudHttpCredsPage *_httpCredsPage; - OwncloudOAuthCredsPage *_browserCredsPage; Flow2AuthCredsPage *_flow2CredsPage; OwncloudAdvancedSetupPage *_advancedSetupPage; OwncloudWizardResultPage *_resultPage = nullptr; diff --git a/src/gui/wizard/owncloudwizardcommon.h b/src/gui/wizard/owncloudwizardcommon.h index 093b81bb1908a..fb118d84f6fd2 100644 --- a/src/gui/wizard/owncloudwizardcommon.h +++ b/src/gui/wizard/owncloudwizardcommon.h @@ -44,7 +44,6 @@ namespace WizardCommon { Page_Welcome, Page_ServerSetup, Page_HttpCreds, - Page_OAuthCreds, Page_Flow2AuthCreds, #ifdef WITH_WEBENGINE Page_WebView, diff --git a/src/libsync/creds/httpcredentials.cpp b/src/libsync/creds/httpcredentials.cpp index 61b74b8c8c192..407f515bac648 100644 --- a/src/libsync/creds/httpcredentials.cpp +++ b/src/libsync/creds/httpcredentials.cpp @@ -39,7 +39,6 @@ Q_LOGGING_CATEGORY(lcHttpCredentials, "nextcloud.sync.credentials.http", QtInfoM namespace { const char userC[] = "user"; - const char isOAuthC[] = "oauth"; const char clientCertBundleC[] = "clientCertPkcs12"; const char clientCertPasswordC[] = "_clientCertPassword"; const char clientCertificatePEMC[] = "_clientCertificatePEM"; @@ -63,14 +62,10 @@ class HttpCredentialsAccessManager : public AccessManager QNetworkRequest req(request); 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); - } + 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 + // Typically the requests to get or refresh the 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); @@ -85,15 +80,7 @@ class HttpCredentialsAccessManager : public AccessManager req.setSslConfiguration(sslConfiguration); } - auto *reply = AccessManager::createRequest(op, req, outgoingData); - - if (_cred->_isRenewingOAuthToken) { - // We know this is going to fail, but we have no way to queue it there, so we will - // simply restart the job after the failure. - reply->setProperty(needRetryC, true); - } - - return reply; + return AccessManager::createRequest(op, req, outgoingData); } private: @@ -178,13 +165,6 @@ void HttpCredentials::fetchFromKeychain() // User must be fetched from config file fetchUser(); - if (!_ready && !_refreshToken.isEmpty()) { - // This happens if the credentials are still loaded from the keychain, but we are called - // here because the auth is invalid, so this means we simply need to refresh the credentials - refreshAccessToken(); - return; - } - if (_ready) { Q_EMIT fetched(); } else { @@ -370,20 +350,13 @@ void HttpCredentials::slotReadJobDone(QKeychain::Job *incoming) return; } - bool isOauth = _account->credentialSetting(QLatin1String(isOAuthC)).toBool(); - if (isOauth) { - _refreshToken = job->textData(); - } else { - _password = job->textData(); - } + _password = job->textData(); if (_user.isEmpty()) { qCWarning(lcHttpCredentials) << "Strange: User is empty!"; } - if (!_refreshToken.isEmpty() && error == QKeychain::NoError) { - refreshAccessToken(); - } else if (!_password.isEmpty() && error == QKeychain::NoError) { + if (!_password.isEmpty() && error == QKeychain::NoError) { // All cool, the keychain did not come back with error. // Still, the password can be empty which indicates a problem and // the password dialog has to be opened. @@ -408,58 +381,6 @@ void HttpCredentials::slotReadJobDone(QKeychain::Job *incoming) } } -bool HttpCredentials::refreshAccessToken() -{ - if (_refreshToken.isEmpty()) - return false; - - QUrl requestToken = Utility::concatUrlPath(_account->url(), QLatin1String("/index.php/apps/oauth2/api/v1/token")); - QNetworkRequest req; - req.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded"); - - QString basicAuth = QString("%1:%2").arg( - Theme::instance()->oauthClientId(), Theme::instance()->oauthClientSecret()); - req.setRawHeader("Authorization", "Basic " + basicAuth.toUtf8().toBase64()); - req.setAttribute(HttpCredentials::DontAddCredentialsAttribute, true); - - auto requestBody = new QBuffer; - QUrlQuery arguments(QString("grant_type=refresh_token&refresh_token=%1").arg(_refreshToken)); - requestBody->setData(arguments.query(QUrl::FullyEncoded).toLatin1()); - - auto job = _account->sendRequest("POST", requestToken, req, requestBody); - job->setTimeout(qMin(30 * 1000ll, job->timeoutMsec())); - QObject::connect(job, &SimpleNetworkJob::finishedSignal, this, [this](QNetworkReply *reply) { - auto jsonData = reply->readAll(); - QJsonParseError jsonParseError{}; - QJsonObject json = QJsonDocument::fromJson(jsonData, &jsonParseError).object(); - QString accessToken = json["access_token"].toString(); - if (jsonParseError.error != QJsonParseError::NoError || json.isEmpty()) { - // Invalid or empty JSON: Network error maybe? - qCWarning(lcHttpCredentials) << "Error while refreshing the token" << reply->errorString() << jsonData << jsonParseError.errorString(); - } else if (accessToken.isEmpty()) { - // If the json was valid, but the reply did not contain an access token, the token - // is considered expired. (Usually the HTTP reply code is 400) - qCDebug(lcHttpCredentials) << "Expired refresh token. Logging out"; - _refreshToken.clear(); - } else { - _ready = true; - _password = accessToken; - _refreshToken = json["refresh_token"].toString(); - persist(); - } - _isRenewingOAuthToken = false; - for (const auto &job : qAsConst(_retryQueue)) { - if (job) - job->retry(); - } - _retryQueue.clear(); - emit fetched(); - }); - _isRenewingOAuthToken = true; - return true; -} - - void HttpCredentials::invalidateToken() { if (!_password.isEmpty()) { @@ -480,12 +401,6 @@ void HttpCredentials::invalidateToken() // clear the session cookie. _account->clearCookieJar(); - if (!_refreshToken.isEmpty()) { - // Only invalidate the access_token (_password) but keep the _refreshToken in the keychain - // (when coming from forgetSensitiveData, the _refreshToken is cleared) - return; - } - auto *job = new QKeychain::DeletePasswordJob(Theme::instance()->appName()); addSettingsToJob(_account, job); job->setInsecureFallback(true); @@ -502,9 +417,6 @@ void HttpCredentials::invalidateToken() void HttpCredentials::forgetSensitiveData() { - // need to be done before invalidateToken, so it actually deletes the refresh_token from the keychain - _refreshToken.clear(); - invalidateToken(); _previousPassword.clear(); } @@ -517,7 +429,6 @@ void HttpCredentials::persist() } _account->setCredentialSetting(QLatin1String(userC), _user); - _account->setCredentialSetting(QLatin1String(isOAuthC), isUsingOAuth()); if (!_clientCertBundle.isEmpty()) { // Note that the _clientCertBundle will often be cleared after usage, // it's just written if it gets passed into the constructor. @@ -605,7 +516,7 @@ void HttpCredentials::slotWritePasswordToKeychain() job->setInsecureFallback(false); connect(job, &QKeychain::Job::finished, this, &HttpCredentials::slotWriteJobDone); job->setKey(keychainKey(_account->url().toString(), _user, _account->id())); - job->setTextData(isUsingOAuth() ? _refreshToken : _password); + job->setTextData(_password); job->start(); } @@ -621,19 +532,12 @@ void HttpCredentials::slotAuthentication(QNetworkReply *reply, QAuthenticator *a { if (!_ready) return; + Q_UNUSED(authenticator) // Because of issue #4326, we need to set the login and password manually at every requests // Thus, if we reach this signal, those credentials were invalid and we terminate. qCWarning(lcHttpCredentials) << "Stop request: Authentication failed for " << reply->url().toString(); reply->setProperty(authenticationFailedC, true); - - if (_isRenewingOAuthToken) { - reply->setProperty(needRetryC, true); - } else if (isUsingOAuth() && !reply->property(needRetryC).toBool()) { - reply->setProperty(needRetryC, true); - qCInfo(lcHttpCredentials) << "Refreshing token"; - refreshAccessToken(); - } } bool HttpCredentials::retryIfNeeded(AbstractNetworkJob *job) @@ -641,11 +545,8 @@ bool HttpCredentials::retryIfNeeded(AbstractNetworkJob *job) auto *reply = job->reply(); if (!reply || !reply->property(needRetryC).toBool()) return false; - if (_isRenewingOAuthToken) { - _retryQueue.append(job); - } else { - job->retry(); - } + + job->retry(); return true; } diff --git a/src/libsync/creds/httpcredentials.h b/src/libsync/creds/httpcredentials.h index a2dbb5c1f36df..9c343e5d8026c 100644 --- a/src/libsync/creds/httpcredentials.h +++ b/src/libsync/creds/httpcredentials.h @@ -40,7 +40,7 @@ namespace OCC { HttpCredentials is then split in HttpCredentials and HttpCredentialsGui. - This class handle both HTTP Basic Auth and OAuth. But anything that needs GUI to ask the user + This class handles HTTP Basic Auth. But anything that needs GUI to ask the user is in HttpCredentialsGui. The authentication mechanism looks like this. @@ -52,13 +52,13 @@ namespace OCC { v } slotReadClientCertPEMJobDone } There are first 3 QtKeychain jobs to fetch | } the TLS client keys, if any, and the password - v } (or refresh token + v } slotReadClientKeyPEMJobDone } | } v slotReadJobDone | | - | +-------> emit fetched() if OAuth is not used + | +-------> emit fetched() | v refreshAccessToken() @@ -97,17 +97,9 @@ class OWNCLOUDSYNC_EXPORT HttpCredentials : public AbstractCredentials QString fetchUser(); virtual bool sslIsTrusted() { return false; } - /* If we still have a valid refresh token, try to refresh it assynchronously and emit fetched() - * otherwise return false - */ - bool refreshAccessToken(); - // To fetch the user name as early as possible void setAccount(Account *account) override; - // Whether we are using OAuth - [[nodiscard]] bool isUsingOAuth() const { return !_refreshToken.isNull(); } - bool retryIfNeeded(AbstractNetworkJob *) override; private Q_SLOTS: @@ -157,21 +149,18 @@ private Q_SLOTS: bool unpackClientCertBundle(); QString _user; - QString _password; // user's password, or access_token for OAuth - QString _refreshToken; // OAuth _refreshToken, set if OAuth is used. + QString _password; // user's password QString _previousPassword; QString _fetchErrorString; bool _ready = false; - bool _isRenewingOAuthToken = false; + QByteArray _clientCertBundle; QByteArray _clientCertPassword; QSslKey _clientSslKey; QSslCertificate _clientSslCertificate; bool _keychainMigration = false; bool _retryOnKeyChainError = true; // true if we haven't done yet any reading from keychain - - QVector> _retryQueue; // Jobs we need to retry once the auth token is fetched }; diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp index ddbc08b3be285..8c4a9d21ae203 100644 --- a/src/libsync/networkjobs.cpp +++ b/src/libsync/networkjobs.cpp @@ -1056,16 +1056,13 @@ void DetermineAuthTypeJob::start() }); connect(propfind, &SimpleNetworkJob::finishedSignal, this, [this](QNetworkReply *reply) { auto authChallenge = reply->rawHeader("WWW-Authenticate").toLower(); - if (authChallenge.contains("bearer ")) { - _resultPropfind = OAuth; + + if (authChallenge.isEmpty()) { + qCWarning(lcDetermineAuthTypeJob) << "Did not receive WWW-Authenticate reply to auth-test PROPFIND"; } else { - if (authChallenge.isEmpty()) { - qCWarning(lcDetermineAuthTypeJob) << "Did not receive WWW-Authenticate reply to auth-test PROPFIND"; - } else { - qCWarning(lcDetermineAuthTypeJob) << "Unknown WWW-Authenticate reply to auth-test PROPFIND:" << authChallenge; - } - _resultPropfind = Basic; + qCWarning(lcDetermineAuthTypeJob) << "Unknown WWW-Authenticate reply to auth-test PROPFIND:" << authChallenge; } + _resultPropfind = Basic; _propfindDone = true; checkAllDone(); }); @@ -1111,13 +1108,13 @@ void DetermineAuthTypeJob::checkAllDone() auto result = _resultPropfind; #ifdef WITH_WEBENGINE - // WebViewFlow > OAuth > Basic + // WebViewFlow > Basic if (_account->serverVersionInt() >= Account::makeServerVersion(12, 0, 0)) { result = WebViewFlow; } #endif // WITH_WEBENGINE - // LoginFlowV2 > WebViewFlow > OAuth > Basic + // LoginFlowV2 > WebViewFlow > Basic if (_account->serverVersionInt() >= Account::makeServerVersion(16, 0, 0)) { result = LoginFlowV2; } diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h index 8682a597a8e0a..4fc397a7a4129 100644 --- a/src/libsync/networkjobs.h +++ b/src/libsync/networkjobs.h @@ -523,7 +523,6 @@ class OWNCLOUDSYNC_EXPORT DetermineAuthTypeJob : public QObject WebViewFlow, #endif // WITH_WEBENGINE Basic, // also the catch-all fallback for backwards compatibility reasons - OAuth, LoginFlowV2 }; Q_ENUM(AuthType) diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 08ab484891501..f2016dc1a7167 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -163,7 +163,7 @@ void GETFileJob::slotMetaDataChanged() if (httpStatus == 301 || httpStatus == 302 || httpStatus == 303 || httpStatus == 307 || httpStatus == 308 || httpStatus == 401) { - // Redirects and auth failures (oauth token renew) are handled by AbstractNetworkJob and + // Redirects and auth failures (token renew) are handled by AbstractNetworkJob and // will end up restarting the job. We do not want to process further data from the initial // request. newReplyHook() will reestablish signal connections for the follow-up request. bool ok = disconnect(reply(), &QNetworkReply::finished, this, &GETFileJob::slotReadyRead) diff --git a/src/libsync/theme.cpp b/src/libsync/theme.cpp index fb589c29d601c..bb930494ea9db 100644 --- a/src/libsync/theme.cpp +++ b/src/libsync/theme.cpp @@ -783,16 +783,6 @@ QString Theme::quotaBaseFolder() const return QLatin1String("/"); } -QString Theme::oauthClientId() const -{ - return "xdXOt13JKxym1B1QcEncf2XDkLAexMBFwiT9j6EfhhHFJhs2KM9jbjTmf8JBXE69"; -} - -QString Theme::oauthClientSecret() const -{ - return "UBntmLjC2yYCeHwsyj73Uwo9TAaecAetRwMw0xYcvNL9yRdLSUi0hUAHfvCHFeFh"; -} - QString Theme::versionSwitchOutput() const { QString helpText; diff --git a/src/libsync/theme.h b/src/libsync/theme.h index 55120652f01b2..cb06e1263bf3f 100644 --- a/src/libsync/theme.h +++ b/src/libsync/theme.h @@ -466,13 +466,6 @@ class OWNCLOUDSYNC_EXPORT Theme : public QObject */ [[nodiscard]] QString quotaBaseFolder() const; - /** - * The OAuth client_id, secret pair. - * Note that client that change these value cannot connect to un-branded owncloud servers. - */ - [[nodiscard]] QString oauthClientId() const; - [[nodiscard]] QString oauthClientSecret() const; - /** * @brief What should be output for the --version command line switch. * diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 69ebc08d518b4..cd2ff20ae019b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -114,8 +114,6 @@ nextcloud_add_test(Account) nextcloud_add_test(FolderMan) nextcloud_add_test(RemoteWipe) -nextcloud_add_test(OAuth) - configure_file(test_journal.db "${PROJECT_BINARY_DIR}/bin/test_journal.db" COPYONLY) find_package(CMocka) diff --git a/test/testoauth.cpp b/test/testoauth.cpp deleted file mode 100644 index d3be8dcffb2b6..0000000000000 --- a/test/testoauth.cpp +++ /dev/null @@ -1,337 +0,0 @@ -/* - * This software is in the public domain, furnished "as is", without technical - * support, and with no warranty, express or implied, as to its usefulness for - * any purpose. - * - */ - -#include -#include - -#include "gui/creds/oauth.h" -#include "syncenginetestutils.h" -#include "theme.h" -#include "common/asserts.h" - -using namespace OCC; - -class DesktopServiceHook : public QObject -{ - Q_OBJECT -signals: - void hooked(const QUrl &); -public: - DesktopServiceHook() { QDesktopServices::setUrlHandler("oauthtest", this, "hooked"); } -}; - -static const QUrl sOAuthTestServer("oauthtest://someserver/owncloud"); - - -class FakePostReply : public QNetworkReply -{ - Q_OBJECT -public: - std::unique_ptr payload; - bool aborted = false; - bool redirectToPolicy = false; - bool redirectToToken = false; - - FakePostReply(QNetworkAccessManager::Operation op, const QNetworkRequest &request, - std::unique_ptr payload_, QObject *parent) - : QNetworkReply{parent}, payload{std::move(payload_)} - { - setRequest(request); - setUrl(request.url()); - setOperation(op); - open(QIODevice::ReadOnly); - payload->open(QIODevice::ReadOnly); - QMetaObject::invokeMethod(this, "respond", Qt::QueuedConnection); - } - - Q_INVOKABLE virtual void respond() { - if (aborted) { - setError(OperationCanceledError, "Operation Canceled"); - emit metaDataChanged(); - emit finished(); - return; - } else if (redirectToPolicy) { - setHeader(QNetworkRequest::LocationHeader, "/my.policy"); - setAttribute(QNetworkRequest::RedirectionTargetAttribute, "/my.policy"); - setAttribute(QNetworkRequest::HttpStatusCodeAttribute, 302); // 302 might or might not lose POST data in rfc - setHeader(QNetworkRequest::ContentLengthHeader, 0); - emit metaDataChanged(); - emit finished(); - return; - } else if (redirectToToken) { - // Redirect to self - QVariant destination = QVariant(sOAuthTestServer.toString()+QLatin1String("/index.php/apps/oauth2/api/v1/token")); - setHeader(QNetworkRequest::LocationHeader, destination); - setAttribute(QNetworkRequest::RedirectionTargetAttribute, destination); - setAttribute(QNetworkRequest::HttpStatusCodeAttribute, 307); // 307 explicitly in rfc says to not lose POST data - setHeader(QNetworkRequest::ContentLengthHeader, 0); - emit metaDataChanged(); - emit finished(); - return; - } - setHeader(QNetworkRequest::ContentLengthHeader, payload->size()); - setAttribute(QNetworkRequest::HttpStatusCodeAttribute, 200); - emit metaDataChanged(); - if (bytesAvailable()) - emit readyRead(); - emit finished(); - } - - void abort() override { - aborted = true; - } - [[nodiscard]] qint64 bytesAvailable() const override { - if (aborted) - return 0; - return payload->bytesAvailable(); - } - - qint64 readData(char *data, qint64 maxlen) override { - return payload->read(data, maxlen); - } -}; - -// Reply with a small delay -class SlowFakePostReply : public FakePostReply { - Q_OBJECT -public: - using FakePostReply::FakePostReply; - void respond() override { - // override of FakePostReply::respond, will call the real one with a delay. - QTimer::singleShot(100, this, [this] { this->FakePostReply::respond(); }); - } -}; - - -class OAuthTestCase : public QObject -{ - Q_OBJECT - DesktopServiceHook desktopServiceHook; -public: - enum State { StartState, BrowserOpened, TokenAsked, CustomState } state = StartState; - Q_ENUM(State); - bool replyToBrowserOk = false; - bool gotAuthOk = false; - [[nodiscard]] virtual bool done() const { return replyToBrowserOk && gotAuthOk; } - - FakeQNAM *fakeQnam = nullptr; - QNetworkAccessManager realQNAM; - QPointer browserReply = nullptr; - QString code = generateEtag(); - OCC::AccountPtr account; - - QScopedPointer oauth; - - virtual void test() { - fakeQnam = new FakeQNAM({}); - account = OCC::Account::create(); - account->setUrl(sOAuthTestServer); - account->setCredentials(new FakeCredentials{fakeQnam}); - fakeQnam->setParent(this); - fakeQnam->setOverride([this](QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *device) { - ASSERT(device); - ASSERT(device->bytesAvailable()>0); // OAuth2 always sends around POST data. - return this->tokenReply(op, req); - }); - - QObject::connect(&desktopServiceHook, &DesktopServiceHook::hooked, - this, &OAuthTestCase::openBrowserHook); - - oauth.reset(new OAuth(account.data(), nullptr)); - QObject::connect(oauth.data(), &OAuth::result, this, &OAuthTestCase::oauthResult); - oauth->start(); - QTRY_VERIFY(done()); - } - - virtual void openBrowserHook(const QUrl &url) { - QCOMPARE(state, StartState); - state = BrowserOpened; - QCOMPARE(url.path(), QString(sOAuthTestServer.path() + "/index.php/apps/oauth2/authorize")); - QVERIFY(url.toString().startsWith(sOAuthTestServer.toString())); - QUrlQuery query(url); - QCOMPARE(query.queryItemValue(QLatin1String("response_type")), QLatin1String("code")); - QCOMPARE(query.queryItemValue(QLatin1String("client_id")), Theme::instance()->oauthClientId()); - QUrl redirectUri(query.queryItemValue(QLatin1String("redirect_uri"))); - QCOMPARE(redirectUri.host(), QLatin1String("localhost")); - redirectUri.setQuery("code=" + code); - createBrowserReply(QNetworkRequest(redirectUri)); - } - - virtual QNetworkReply *createBrowserReply(const QNetworkRequest &request) { - browserReply = realQNAM.get(request); - QObject::connect(browserReply, &QNetworkReply::finished, this, &OAuthTestCase::browserReplyFinished); - return browserReply; - } - - virtual void browserReplyFinished() { - QCOMPARE(sender(), browserReply.data()); - QCOMPARE(state, TokenAsked); - browserReply->deleteLater(); - QCOMPARE(browserReply->rawHeader("Location"), QByteArray("owncloud://success")); - replyToBrowserOk = true; - }; - - virtual QNetworkReply *tokenReply(QNetworkAccessManager::Operation op, const QNetworkRequest &req) - { - ASSERT(state == BrowserOpened); - state = TokenAsked; - ASSERT(op == QNetworkAccessManager::PostOperation); - ASSERT(req.url().toString().startsWith(sOAuthTestServer.toString())); - ASSERT(req.url().path() == sOAuthTestServer.path() + "/index.php/apps/oauth2/api/v1/token"); - std::unique_ptr payload(new QBuffer()); - payload->setData(tokenReplyPayload()); - return new FakePostReply(op, req, std::move(payload), fakeQnam); - } - - [[nodiscard]] virtual QByteArray tokenReplyPayload() const { - QJsonDocument jsondata(QJsonObject{ - { "access_token", "123" }, - { "refresh_token" , "456" }, - { "message_url", "owncloud://success"}, - { "user_id", "789" }, - { "token_type", "Bearer" } - }); - return jsondata.toJson(); - } - - virtual void oauthResult(OAuth::Result result, const QString &user, const QString &token , const QString &refreshToken) { - QCOMPARE(state, TokenAsked); - QCOMPARE(result, OAuth::LoggedIn); - QCOMPARE(user, QString("789")); - QCOMPARE(token, QString("123")); - QCOMPARE(refreshToken, QString("456")); - gotAuthOk = true; - } -}; - -class TestOAuth: public QObject -{ - Q_OBJECT - -private slots: - void testBasic() - { - OAuthTestCase test; - test.test(); - } - - // Test for https://github.com/owncloud/client/pull/6057 - void testCloseBrowserDontCrash() - { - struct Test : OAuthTestCase { - QNetworkReply *tokenReply(QNetworkAccessManager::Operation op, const QNetworkRequest & req) override - { - ASSERT(browserReply); - // simulate the fact that the browser is closing the connection - browserReply->abort(); - QCoreApplication::processEvents(); - - ASSERT(state == BrowserOpened); - state = TokenAsked; - - std::unique_ptr payload(new QBuffer); - payload->setData(tokenReplyPayload()); - return new SlowFakePostReply(op, req, std::move(payload), fakeQnam); - } - - void browserReplyFinished() override - { - QCOMPARE(sender(), browserReply.data()); - QCOMPARE(browserReply->error(), QNetworkReply::OperationCanceledError); - replyToBrowserOk = true; - } - } test; - test.test(); - } - - void testRandomConnections() - { - // Test that we can send random garbage to the litening socket and it does not prevent the connection - struct Test : OAuthTestCase { - QNetworkReply *createBrowserReply(const QNetworkRequest &request) override { - QTimer::singleShot(0, this, [this, request] { - auto port = request.url().port(); - state = CustomState; - QVector payloads = { - "GET FOFOFO HTTP 1/1\n\n", - "GET /?code=invalie HTTP 1/1\n\n", - "GET /?code=xxxxx&bar=fff", - QByteArray("\0\0\0", 3), - QByteArray("GET \0\0\0 \n\n\n\n\n\0", 14), - QByteArray("GET /?code=éléphant\xa5 HTTP\n"), - QByteArray("\n\n\n\n"), - }; - foreach (const auto &x, payloads) { - auto socket = new QTcpSocket(this); - socket->connectToHost("localhost", port); - QVERIFY(socket->waitForConnected()); - socket->write(x); - } - - // Do the actual request a bit later - QTimer::singleShot(100, this, [this, request] { - QCOMPARE(state, CustomState); - state = BrowserOpened; - this->OAuthTestCase::createBrowserReply(request); - }); - }); - return nullptr; - } - - QNetworkReply *tokenReply(QNetworkAccessManager::Operation op, const QNetworkRequest &req) override - { - if (state == CustomState) - return new FakeErrorReply{op, req, this, 500}; - return OAuthTestCase::tokenReply(op, req); - } - - void oauthResult(OAuth::Result result, const QString &user, const QString &token , - const QString &refreshToken) override { - if (state != CustomState) - return OAuthTestCase::oauthResult(result, user, token, refreshToken); - QCOMPARE(result, OAuth::Error); - } - } test; - test.test(); - } - - void testTokenUrlHasRedirect() - { - struct Test : OAuthTestCase { - int redirectsDone = 0; - QNetworkReply *tokenReply(QNetworkAccessManager::Operation op, const QNetworkRequest & request) override - { - ASSERT(browserReply); - // Kind of reproduces what we had in https://github.com/owncloud/enterprise/issues/2951 (not 1:1) - if (redirectsDone == 0) { - std::unique_ptr payload(new QBuffer()); - payload->setData(""); - auto *reply = new SlowFakePostReply(op, request, std::move(payload), this); - reply->redirectToPolicy = true; - redirectsDone++; - return reply; - } else if (redirectsDone == 1) { - std::unique_ptr payload(new QBuffer()); - payload->setData(""); - auto *reply = new SlowFakePostReply(op, request, std::move(payload), this); - reply->redirectToToken = true; - redirectsDone++; - return reply; - } else { - // ^^ This is with a custom reply and not actually HTTP, so we're testing the HTTP redirect code - // we have in AbstractNetworkJob::slotFinished() - redirectsDone++; - return OAuthTestCase::tokenReply(op, request); - } - } - } test; - test.test(); - } -}; - -QTEST_GUILESS_MAIN(TestOAuth) -#include "testoauth.moc"