From 580f512c945265c45902f202731489de20abc10c Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 12 Oct 2022 18:02:26 +0200 Subject: [PATCH 1/3] introduce a very simple network job to do API requests Signed-off-by: Matthieu Gallien --- src/libsync/networkjobs.cpp | 57 +++++++++++++++++++++++++++++++++++++ src/libsync/networkjobs.h | 36 +++++++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp index 91970b5ced62..81d43ffb343a 100644 --- a/src/libsync/networkjobs.cpp +++ b/src/libsync/networkjobs.cpp @@ -54,6 +54,7 @@ Q_LOGGING_CATEGORY(lcAvatarJob, "nextcloud.sync.networkjob.avatar", QtInfoMsg) Q_LOGGING_CATEGORY(lcMkColJob, "nextcloud.sync.networkjob.mkcol", QtInfoMsg) Q_LOGGING_CATEGORY(lcProppatchJob, "nextcloud.sync.networkjob.proppatch", QtInfoMsg) Q_LOGGING_CATEGORY(lcJsonApiJob, "nextcloud.sync.networkjob.jsonapi", QtInfoMsg) +Q_LOGGING_CATEGORY(lcSimpleApiJob, "nextcloud.sync.networkjob.simpleapi", QtInfoMsg) Q_LOGGING_CATEGORY(lcDetermineAuthTypeJob, "nextcloud.sync.networkjob.determineauthtype", QtInfoMsg) Q_LOGGING_CATEGORY(lcSimpleFileJob, "nextcloud.sync.networkjob.simplefilejob", QtInfoMsg) const int notModifiedStatusCode = 304; @@ -1183,4 +1184,60 @@ void fetchPrivateLinkUrl(AccountPtr account, const QString &remotePath, job->start(); } +SimpleApiJob::SimpleApiJob(const AccountPtr &account, const QString &path, QObject *parent) + : AbstractNetworkJob(account, path, parent) +{ +} + +void SimpleApiJob::setBody(const QByteArray &body) +{ + _body = body; + qCDebug(lcSimpleApiJob) << "Set body for request:" << _body; +} + + +void SimpleApiJob::setVerb(Verb value) +{ + _verb = value; +} + + +QByteArray SimpleApiJob::verbToString() const +{ + switch (_verb) { + case Verb::Get: + return "GET"; + case Verb::Post: + return "POST"; + case Verb::Put: + return "PUT"; + case Verb::Delete: + return "DELETE"; + } + return "GET"; +} + +void SimpleApiJob::start() +{ + qCDebug(lcSimpleApiJob) << "send: " << path() << _body; + + _request.setRawHeader("OCS-APIREQUEST", "true"); + const auto url = Utility::concatUrlPath(account()->url(), path()); + const auto httpVerb = verbToString(); + if (!_body.isEmpty()) { + sendRequest(httpVerb, url, _request, _body); + } else { + sendRequest(httpVerb, url, _request); + } + AbstractNetworkJob::start(); +} + +bool SimpleApiJob::finished() +{ + const auto httpStatusCode = reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); + qCDebug(lcSimpleApiJob) << "result: " << path() << errorString() << httpStatusCode; + emit resultReceived(httpStatusCode); + return true; +} + } // namespace OCC diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h index 644244a10a2a..c89368c45c3d 100644 --- a/src/libsync/networkjobs.h +++ b/src/libsync/networkjobs.h @@ -459,6 +459,42 @@ public slots: [[nodiscard]] QByteArray verbToString() const; }; +class OWNCLOUDSYNC_EXPORT SimpleApiJob : public AbstractNetworkJob +{ + Q_OBJECT +public: + enum class Verb { + Get, + Post, + Put, + Delete, + }; + + explicit SimpleApiJob(const AccountPtr &account, const QString &path, QObject *parent = nullptr); + + void setBody(const QByteArray &body); + + void setVerb(Verb value); + +public slots: + void start() override; + +Q_SIGNALS: + + void resultReceived(int statusCode); + +protected: + bool finished() override; + +private: + QByteArray _body; + QNetworkRequest _request; + + Verb _verb = Verb::Get; + + [[nodiscard]] QByteArray verbToString() const; +}; + /** * @brief Checks with auth type to use for a server * @ingroup libsync From 615c02e3d3ee5daa62f8780e43d28bb631b55df8 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 13 Oct 2022 12:14:22 +0200 Subject: [PATCH 2/3] makes JsonApiJob share common code with SimpleApiJob Signed-off-by: Matthieu Gallien --- src/libsync/networkjobs.cpp | 93 +++++++++++++----------------- src/libsync/networkjobs.h | 109 ++++++++++++++++-------------------- 2 files changed, 85 insertions(+), 117 deletions(-) diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp index 81d43ffb343a..a2a17567f385 100644 --- a/src/libsync/networkjobs.cpp +++ b/src/libsync/networkjobs.cpp @@ -823,64 +823,23 @@ bool EntityExistsJob::finished() /*********************************************************************************************/ JsonApiJob::JsonApiJob(const AccountPtr &account, const QString &path, QObject *parent) - : AbstractNetworkJob(account, path, parent) -{ -} - -void JsonApiJob::addQueryParams(const QUrlQuery ¶ms) -{ - _additionalParams = params; -} - -void JsonApiJob::addRawHeader(const QByteArray &headerName, const QByteArray &value) + : SimpleApiJob(account, path, parent) { - _request.setRawHeader(headerName, value); } void JsonApiJob::setBody(const QJsonDocument &body) { - _body = body.toJson(); - qCDebug(lcJsonApiJob) << "Set body for request:" << _body; - if (!_body.isEmpty()) { - _request.setHeader(QNetworkRequest::ContentTypeHeader, "application/json"); - } -} - - -void JsonApiJob::setVerb(Verb value) -{ - _verb = value; -} - - -QByteArray JsonApiJob::verbToString() const -{ - switch (_verb) { - case Verb::Get: - return "GET"; - case Verb::Post: - return "POST"; - case Verb::Put: - return "PUT"; - case Verb::Delete: - return "DELETE"; + SimpleApiJob::setBody(body.toJson()); + qCDebug(lcJsonApiJob) << "Set body for request:" << SimpleApiJob::body(); + if (!SimpleApiJob::body().isEmpty()) { + request().setHeader(QNetworkRequest::ContentTypeHeader, "application/json"); } - return "GET"; } void JsonApiJob::start() { - addRawHeader("OCS-APIREQUEST", "true"); - auto query = _additionalParams; - query.addQueryItem(QLatin1String("format"), QLatin1String("json")); - QUrl url = Utility::concatUrlPath(account()->url(), path(), query); - const auto httpVerb = verbToString(); - if (!_body.isEmpty()) { - sendRequest(httpVerb, url, _request, _body); - } else { - sendRequest(httpVerb, url, _request); - } - AbstractNetworkJob::start(); + additionalParams().addQueryItem(QLatin1String("format"), QLatin1String("json")); + SimpleApiJob::start(); } bool JsonApiJob::finished() @@ -1219,15 +1178,14 @@ QByteArray SimpleApiJob::verbToString() const void SimpleApiJob::start() { - qCDebug(lcSimpleApiJob) << "send: " << path() << _body; - - _request.setRawHeader("OCS-APIREQUEST", "true"); - const auto url = Utility::concatUrlPath(account()->url(), path()); + addRawHeader("OCS-APIREQUEST", "true"); + auto query = _additionalParams; + QUrl url = Utility::concatUrlPath(account()->url(), path(), query); const auto httpVerb = verbToString(); - if (!_body.isEmpty()) { - sendRequest(httpVerb, url, _request, _body); + if (!SimpleApiJob::body().isEmpty()) { + sendRequest(httpVerb, url, request(), SimpleApiJob::body()); } else { - sendRequest(httpVerb, url, _request); + sendRequest(httpVerb, url, request()); } AbstractNetworkJob::start(); } @@ -1240,4 +1198,29 @@ bool SimpleApiJob::finished() return true; } +QNetworkRequest& SimpleApiJob::request() +{ + return _request; +} + +QByteArray& SimpleApiJob::body() +{ + return _body; +} + +QUrlQuery &SimpleApiJob::additionalParams() +{ + return _additionalParams; +} + +void SimpleApiJob::addQueryParams(const QUrlQuery ¶ms) +{ + _additionalParams = params; +} + +void SimpleApiJob::addRawHeader(const QByteArray &headerName, const QByteArray &value) +{ + request().setRawHeader(headerName, value); +} + } // namespace OCC diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h index c89368c45c3d..1416e69315ff 100644 --- a/src/libsync/networkjobs.h +++ b/src/libsync/networkjobs.h @@ -382,22 +382,7 @@ private slots: bool finished() override; }; -/** - * @brief Job to check an API that return JSON - * - * Note! you need to be in the connected state before calling this because of a server bug: - * https://github.com/owncloud/core/issues/12930 - * - * To be used like this: - * \code - * _job = new JsonApiJob(account, QLatin1String("ocs/v1.php/foo/bar"), this); - * connect(job, SIGNAL(jsonReceived(QJsonDocument)), ...) - * The received QVariantMap is null in case of error - * \encode - * - * @ingroup libsync - */ -class OWNCLOUDSYNC_EXPORT JsonApiJob : public AbstractNetworkJob +class OWNCLOUDSYNC_EXPORT SimpleApiJob : public AbstractNetworkJob { Q_OBJECT public: @@ -406,9 +391,13 @@ class OWNCLOUDSYNC_EXPORT JsonApiJob : public AbstractNetworkJob Post, Put, Delete, - }; + }; - explicit JsonApiJob(const AccountPtr &account, const QString &path, QObject *parent = nullptr); + explicit SimpleApiJob(const AccountPtr &account, const QString &path, QObject *parent = nullptr); + + void setBody(const QByteArray &body); + + void setVerb(Verb value); /** * @brief addQueryParams - add more parameters to the ocs call @@ -423,76 +412,72 @@ class OWNCLOUDSYNC_EXPORT JsonApiJob : public AbstractNetworkJob void addQueryParams(const QUrlQuery ¶ms); void addRawHeader(const QByteArray &headerName, const QByteArray &value); - void setBody(const QJsonDocument &body); - - void setVerb(Verb value); - public slots: void start() override; +Q_SIGNALS: + + void resultReceived(int statusCode); + protected: bool finished() override; -signals: - - /** - * @brief jsonReceived - signal to report the json answer from ocs - * @param json - the parsed json document - * @param statusCode - the OCS status code: 100 (!) for success - */ - void jsonReceived(const QJsonDocument &json, int statusCode); - /** - * @brief etagResponseHeaderReceived - signal to report the ETag response header value - * from ocs api v2 - * @param value - the ETag response header value - * @param statusCode - the OCS status code: 100 (!) for success - */ - void etagResponseHeaderReceived(const QByteArray &value, int statusCode); + [[nodiscard]] QNetworkRequest& request(); + [[nodiscard]] QByteArray& body(); + [[nodiscard]] QUrlQuery& additionalParams(); + [[nodiscard]] QByteArray verbToString() const; private: QByteArray _body; QUrlQuery _additionalParams; QNetworkRequest _request; - Verb _verb = Verb::Get; - - [[nodiscard]] QByteArray verbToString() const; }; -class OWNCLOUDSYNC_EXPORT SimpleApiJob : public AbstractNetworkJob +/** + * @brief Job to check an API that return JSON + * + * Note! you need to be in the connected state before calling this because of a server bug: + * https://github.com/owncloud/core/issues/12930 + * + * To be used like this: + * \code + * _job = new JsonApiJob(account, QLatin1String("ocs/v1.php/foo/bar"), this); + * connect(job, SIGNAL(jsonReceived(QJsonDocument)), ...) + * The received QVariantMap is null in case of error + * \encode + * + * @ingroup libsync + */ +class OWNCLOUDSYNC_EXPORT JsonApiJob : public SimpleApiJob { Q_OBJECT public: - enum class Verb { - Get, - Post, - Put, - Delete, - }; - - explicit SimpleApiJob(const AccountPtr &account, const QString &path, QObject *parent = nullptr); - - void setBody(const QByteArray &body); + explicit JsonApiJob(const AccountPtr &account, const QString &path, QObject *parent = nullptr); - void setVerb(Verb value); + void setBody(const QJsonDocument &body); public slots: void start() override; -Q_SIGNALS: - - void resultReceived(int statusCode); - protected: bool finished() override; +signals: -private: - QByteArray _body; - QNetworkRequest _request; - - Verb _verb = Verb::Get; + /** + * @brief jsonReceived - signal to report the json answer from ocs + * @param json - the parsed json document + * @param statusCode - the OCS status code: 100 (!) for success + */ + void jsonReceived(const QJsonDocument &json, int statusCode); - [[nodiscard]] QByteArray verbToString() const; + /** + * @brief etagResponseHeaderReceived - signal to report the ETag response header value + * from ocs api v2 + * @param value - the ETag response header value + * @param statusCode - the OCS status code: 100 (!) for success + */ + void etagResponseHeaderReceived(const QByteArray &value, int statusCode); }; /** From f9949ee0de1b8a42412f528dc1f64e080eb44316 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 12 Oct 2022 18:01:15 +0200 Subject: [PATCH 3/3] edit locally requires a valid token check on server that the token received during a request to open a local file is indeed a valid one Signed-off-by: Matthieu Gallien --- src/gui/application.cpp | 11 ++++++++- src/gui/folderman.cpp | 55 ++++++++++++++++++++++++++++------------- src/gui/folderman.h | 2 +- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 21cce1a67c74..6f891e022edf 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -61,6 +61,7 @@ #include #include #include +#include class QSocket; @@ -764,8 +765,16 @@ void Application::handleEditLocally(const QUrl &url) const // for a sample URL "nc://open/admin@nextcloud.lan:8080/Photos/lovely.jpg", QUrl::path would return "admin@nextcloud.lan:8080/Photos/lovely.jpg" const auto accountDisplayName = pathSplit.takeFirst(); const auto fileRemotePath = pathSplit.join('/'); + const auto urlQuery = QUrlQuery{url}; - FolderMan::instance()->editFileLocally(accountDisplayName, fileRemotePath); + auto token = QString{}; + if (urlQuery.hasQueryItem(QStringLiteral("token"))) { + token = urlQuery.queryItemValue(QStringLiteral("token")); + } else { + qCWarning(lcApplication) << "Invalid URL for file local editing: missing token"; + } + + FolderMan::instance()->editFileLocally(accountDisplayName, fileRemotePath, token); } QString substLang(const QString &lang) diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 220313891dd7..d76a4fccd0ce 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -1422,7 +1422,7 @@ void FolderMan::setDirtyNetworkLimits() } } -void FolderMan::editFileLocally(const QString &accountDisplayName, const QString &relPath) +void FolderMan::editFileLocally(const QString &accountDisplayName, const QString &relPath, const QString &token) { const auto showError = [this](const OCC::AccountStatePtr accountState, const QString &errorMessage, const QString &subject) { if (accountState && accountState->account()) { @@ -1447,6 +1447,12 @@ void FolderMan::editFileLocally(const QString &accountDisplayName, const QString messageBox->raise(); }; + if (token.isEmpty()) { + qCWarning(lcFolderMan) << "Edit locally request is missing a valid token. Impossible to open the file."; + showError({}, tr("Edit locally request is not valid. Opening the file is forbidden."), accountDisplayName); + return; + } + const auto accountFound = AccountManager::instance()->account(accountDisplayName); if (!accountFound) { @@ -1488,23 +1494,38 @@ void FolderMan::editFileLocally(const QString &accountDisplayName, const QString showError(accountFound, tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); return; } - folderForFile->startSync(); - _localFileEditingSyncFinishedConnections.insert(localFilePath, QObject::connect(folderForFile, &Folder::syncFinished, this, - [this, localFilePath](const OCC::SyncResult &result) { - Q_UNUSED(result); - const auto foundConnectionIt = _localFileEditingSyncFinishedConnections.find(localFilePath); - if (foundConnectionIt != std::end(_localFileEditingSyncFinishedConnections) && foundConnectionIt.value()) { - QObject::disconnect(foundConnectionIt.value()); - _localFileEditingSyncFinishedConnections.erase(foundConnectionIt); - } - // In case the VFS mode is enabled and a file is not yet hydrated, we must call QDesktopServices::openUrl - // from a separate thread, or, there will be a freeze. To avoid searching for a specific folder and checking - // if the VFS is enabled - we just always call it from a separate thread. - QtConcurrent::run([localFilePath]() { - QDesktopServices::openUrl(QUrl::fromLocalFile(localFilePath)); + + const auto checkTokenForEditLocally = new SimpleApiJob(accountFound->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(token)); + checkTokenForEditLocally->setVerb(SimpleApiJob::Verb::Post); + checkTokenForEditLocally->setBody(QByteArray{"path=/"}.append(relPath.toUtf8())); + connect(checkTokenForEditLocally, &SimpleApiJob::resultReceived, checkTokenForEditLocally, [this, folderForFile, localFilePath, showError, accountFound, relPath] (int statusCode) { + constexpr auto HTTP_OK_CODE = 200; + if (statusCode != HTTP_OK_CODE) { Systray::instance()->destroyEditFileLocallyLoadingDialog(); - }); - })); + showError(accountFound, tr("Could not validate the request to open a file from server."), relPath); + qCInfo(lcFolderMan()) << "token check result" << statusCode; + return; + } + + folderForFile->startSync(); + _localFileEditingSyncFinishedConnections.insert(localFilePath, QObject::connect(folderForFile, &Folder::syncFinished, this, + [this, localFilePath](const OCC::SyncResult &result) { + Q_UNUSED(result); + const auto foundConnectionIt = _localFileEditingSyncFinishedConnections.find(localFilePath); + if (foundConnectionIt != std::end(_localFileEditingSyncFinishedConnections) && foundConnectionIt.value()) { + QObject::disconnect(foundConnectionIt.value()); + _localFileEditingSyncFinishedConnections.erase(foundConnectionIt); + } + // In case the VFS mode is enabled and a file is not yet hydrated, we must call QDesktopServices::openUrl + // from a separate thread, or, there will be a freeze. To avoid searching for a specific folder and checking + // if the VFS is enabled - we just always call it from a separate thread. + QtConcurrent::run([localFilePath]() { + QDesktopServices::openUrl(QUrl::fromLocalFile(localFilePath)); + Systray::instance()->destroyEditFileLocallyLoadingDialog(); + }); + })); + }); + checkTokenForEditLocally->start(); } void FolderMan::trayOverallStatus(const QList &folders, diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 77a6b6682b0d..2c9b920e6db4 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -214,7 +214,7 @@ class FolderMan : public QObject void setDirtyNetworkLimits(); /** opens a file with default app, if the file is present **/ - void editFileLocally(const QString &accountDisplayName, const QString &relPath); + void editFileLocally(const QString &accountDisplayName, const QString &relPath, const QString &token); signals: /**