From: Claudio Cambra Date: Fri, 28 Oct 2022 14:42:57 +0000 (+0200) Subject: Verify token before checking disk or modifying state X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~11^2~177^2~5 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=9438d3f1c7bf9b7043b7681f159a0ef749177192;p=nextcloud-desktop.git Verify token before checking disk or modifying state Signed-off-by: Claudio Cambra --- diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 8538a59b0..3c53314a6 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -777,12 +777,14 @@ void Application::handleEditLocally(const QUrl &url) const // We need to make sure the handler sticks around until it is finished const auto editLocallyHandler = new EditLocallyHandler(userId, fileRemotePath, token); - if (editLocallyHandler->ready()) { - connect(editLocallyHandler, &EditLocallyHandler::finished, this, [&editLocallyHandler] { delete editLocallyHandler; }); - editLocallyHandler->startEditLocally(); - } else { - delete editLocallyHandler; - } + const auto editLocallyHandlerDeleter = [editLocallyHandler]{ delete editLocallyHandler; }; + connect(editLocallyHandler, &EditLocallyHandler::error, this, editLocallyHandlerDeleter); + connect(editLocallyHandler, &EditLocallyHandler::fileOpened, this, editLocallyHandlerDeleter); + + connect(editLocallyHandler, &EditLocallyHandler::setupFinished, + editLocallyHandler, [editLocallyHandler]{ editLocallyHandler->startEditLocally(); }); + editLocallyHandler->startSetup(); + } QString substLang(const QString &lang) diff --git a/src/gui/editlocallyhandler.cpp b/src/gui/editlocallyhandler.cpp index 3bacf816c..65f12911c 100644 --- a/src/gui/editlocallyhandler.cpp +++ b/src/gui/editlocallyhandler.cpp @@ -35,36 +35,101 @@ EditLocallyHandler::EditLocallyHandler(const QString &userId, const QString &token, QObject *parent) : QObject{parent} + , _userId(userId) , _relPath(relPath) , _token(token) { - _accountState = AccountManager::instance()->accountFromUserId(userId); +} - if (!_accountState) { - qCWarning(lcEditLocallyHandler) << "Could not find an account " << userId << " to edit file " << relPath << " locally."; - showError(tr("Could not find an account for local editing"), userId); +void EditLocallyHandler::startSetup() +{ + if (_token.isEmpty() || _relPath.isEmpty() || _userId.isEmpty()) { + qCWarning(lcEditLocallyHandler) << "Could not start setup." + << "token:" << _token + << "relPath:" << _relPath + << "userId" << _userId; return; } - if (!isTokenValid(token)) { - qCWarning(lcEditLocallyHandler) << "Edit locally request is missing a valid token, will not open file. Token received was:" << token; + // We check the input data locally first, without modifying any state or + // showing any potentially misleading data to the user + if (!isTokenValid(_token)) { + qCWarning(lcEditLocallyHandler) << "Edit locally request is missing a valid token, will not open file. " + << "Token received was:" << _token; showError(tr("Invalid token received."), tr("Please try again.")); return; } - if (!isRelPathValid(relPath)) { - qCWarning(lcEditLocallyHandler) << "Provided relPath was:" << relPath << "which is not canonical."; + if (!isRelPathValid(_relPath)) { + qCWarning(lcEditLocallyHandler) << "Provided relPath was:" << _relPath << "which is not canonical."; showError(tr("Invalid file path was provided."), tr("Please try again.")); return; } - const auto foundFiles = FolderMan::instance()->findFileInLocalFolders(relPath, _accountState->account()); + _accountState = AccountManager::instance()->accountFromUserId(_userId); + + if (!_accountState) { + qCWarning(lcEditLocallyHandler) << "Could not find an account " << _userId << " to edit file " << _relPath << " locally."; + showError(tr("Could not find an account for local editing."), _userId); + return; + } + + // We now ask the server to verify the token, before we again modify any + // state or look at local files + startTokenRemoteCheck(); +} + +void EditLocallyHandler::startTokenRemoteCheck() +{ + if (!_accountState || _relPath.isEmpty() || _token.isEmpty()) { + qCWarning(lcEditLocallyHandler) << "Could not start token check." + << "accountState:" << _accountState + << "relPath:" << _relPath + << "token:" << _token; + return; + } + + const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(_token)); // Sanitise the token + const auto encodedRelPath = QUrl::toPercentEncoding(_relPath); // Sanitise the relPath + const auto checkEditLocallyToken = new SimpleApiJob(_accountState->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); + + QUrlQuery params; + params.addQueryItem(QStringLiteral("path"), prefixSlashToPath(encodedRelPath)); + checkEditLocallyToken->addQueryParams(params); + checkEditLocallyToken->setVerb(SimpleApiJob::Verb::Post); + connect(checkEditLocallyToken, &SimpleApiJob::resultReceived, this, &EditLocallyHandler::remoteTokenCheckResultReceived); + + checkEditLocallyToken->start(); +} + +void EditLocallyHandler::remoteTokenCheckResultReceived(const int statusCode) +{ + qCInfo(lcEditLocallyHandler) << "token check result" << statusCode; + + constexpr auto HTTP_OK_CODE = 200; + _tokenVerified = statusCode == HTTP_OK_CODE; + + if (!_tokenVerified) { + showError(tr("Could not validate the request to open a file from server."), _relPath); + } + + proceedWithSetup(); +} + +void EditLocallyHandler::proceedWithSetup() +{ + if (!_tokenVerified) { + qCWarning(lcEditLocallyHandler) << "Could not proceed with setup as token is not verified."; + return; + } + + const auto foundFiles = FolderMan::instance()->findFileInLocalFolders(_relPath, _accountState->account()); if (foundFiles.isEmpty()) { - if (isRelPathExcluded(relPath)) { - showError(tr("Could not find a file for local editing. Make sure it is not excluded via selective sync."), relPath); + if (isRelPathExcluded(_relPath)) { + showError(tr("Could not find a file for local editing. Make sure it is not excluded via selective sync."), _relPath); } else { - showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); + showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), _relPath); } return; } @@ -73,24 +138,19 @@ EditLocallyHandler::EditLocallyHandler(const QString &userId, _folderForFile = FolderMan::instance()->folderForPath(_localFilePath); if (!_folderForFile) { - showError(tr("Could not find a folder to sync."), relPath); + showError(tr("Could not find a folder to sync."), _relPath); return; } - const auto relPathSplit = relPath.split(QLatin1Char('/')); - if (relPathSplit.size() == 0) { - showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); + const auto relPathSplit = _relPath.split(QLatin1Char('/')); + if (relPathSplit.isEmpty()) { + showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), _relPath); return; } _fileName = relPathSplit.last(); - _ready = true; -} - -bool EditLocallyHandler::ready() const -{ - return _ready; + Q_EMIT setupFinished(); } QString EditLocallyHandler::prefixSlashToPath(const QString &path) @@ -165,11 +225,12 @@ bool EditLocallyHandler::isRelPathExcluded(const QString &relPath) return true; } -void EditLocallyHandler::showError(const QString &message, const QString &informativeText) const +void EditLocallyHandler::showError(const QString &message, const QString &informativeText) { showErrorNotification(message, informativeText); // to make sure the error is not missed, show a message box in addition showErrorMessageBox(message, informativeText); + Q_EMIT error(message, informativeText); } void EditLocallyHandler::showErrorNotification(const QString &message, const QString &informativeText) const @@ -203,36 +264,16 @@ void EditLocallyHandler::showErrorMessageBox(const QString &message, const QStri void EditLocallyHandler::startEditLocally() { - if (!_ready) { + if (_fileName.isEmpty() || _localFilePath.isEmpty() || !_folderForFile) { + qCWarning(lcEditLocallyHandler) << "Could not start to edit locally." + << "fileName:" << _fileName + << "localFilePath:" << _localFilePath + << "folderForFile:" << _folderForFile; return; } Systray::instance()->createEditFileLocallyLoadingDialog(_fileName); - const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(_token)); // Sanitise the token - const auto encodedRelPath = QUrl::toPercentEncoding(_relPath); // Sanitise the relPath - const auto checkEditLocallyToken = new SimpleApiJob(_accountState->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); - - QUrlQuery params; - params.addQueryItem(QStringLiteral("path"), prefixSlashToPath(_relPath)); - checkEditLocallyToken->addQueryParams(params); - checkEditLocallyToken->setVerb(SimpleApiJob::Verb::Post); - connect(checkEditLocallyToken, &SimpleApiJob::resultReceived, this, &EditLocallyHandler::remoteTokenCheckFinished); - - checkEditLocallyToken->start(); -} - -void EditLocallyHandler::remoteTokenCheckFinished(const int statusCode) -{ - constexpr auto HTTP_OK_CODE = 200; - if (statusCode != HTTP_OK_CODE) { - Systray::instance()->destroyEditFileLocallyLoadingDialog(); - - showError(tr("Could not validate the request to open a file from server."), _relPath); - qCInfo(lcEditLocallyHandler) << "token check result" << statusCode; - return; - } - _folderForFile->startSync(); const auto syncFinishedConnection = connect(_folderForFile, &Folder::syncFinished, this, &EditLocallyHandler::folderSyncFinished); @@ -248,6 +289,10 @@ void EditLocallyHandler::folderSyncFinished(const OCC::SyncResult &result) void EditLocallyHandler::disconnectSyncFinished() const { + if(_localFilePath.isEmpty()) { + return; + } + if (const auto existingConnection = editLocallySyncFinishedConnections.value(_localFilePath)) { disconnect(existingConnection); editLocallySyncFinishedConnections.remove(_localFilePath); @@ -256,6 +301,11 @@ void EditLocallyHandler::disconnectSyncFinished() const void EditLocallyHandler::openFile() { + if(_localFilePath.isEmpty()) { + qCWarning(lcEditLocallyHandler) << "Could not edit locally. Invalid local file path."; + return; + } + const auto localFilePath = _localFilePath; // 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 @@ -265,7 +315,7 @@ void EditLocallyHandler::openFile() Systray::instance()->destroyEditFileLocallyLoadingDialog(); }); - Q_EMIT finished(); + Q_EMIT fileOpened(); } } diff --git a/src/gui/editlocallyhandler.h b/src/gui/editlocallyhandler.h index 9d6f9677c..04791ce93 100644 --- a/src/gui/editlocallyhandler.h +++ b/src/gui/editlocallyhandler.h @@ -36,30 +36,34 @@ public: [[nodiscard]] static bool isRelPathExcluded(const QString &relPath); [[nodiscard]] static QString prefixSlashToPath(const QString &path); - [[nodiscard]] bool ready() const; - signals: - void finished(); + void setupFinished(); + void error(const QString &message, const QString &informativeText); + void fileOpened(); public slots: + void startSetup(); void startEditLocally(); void startTokenRemoteCheck(); private slots: - void showError(const QString &message, const QString &informativeText) const; + void proceedWithSetup(); + + void showError(const QString &message, const QString &informativeText); void showErrorNotification(const QString &message, const QString &informativeText) const; void showErrorMessageBox(const QString &message, const QString &informativeText) const; - void remoteTokenCheckFinished(const int statusCode); + void remoteTokenCheckResultReceived(const int statusCode); void folderSyncFinished(const OCC::SyncResult &result); void disconnectSyncFinished() const; void openFile(); private: - bool _ready = false; + bool _tokenVerified = false; AccountStatePtr _accountState; + QString _userId; QString _relPath; QString _token;