From 772d44c66d60291e59f48e6e99cf3822e2bf2e12 Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Wed, 6 Dec 2023 09:58:17 +0100 Subject: [PATCH 1/3] GUI/LIBSYNC: force login flow V2 with config setting This patch allows a user to set the config variable: forceLoginV2=true This will make the client use the browser login flow instead of the webview. This is useful for making the user experience equal on Windows, Linux and Mac. Currently only Macs use the v2 flow and it was only possible to get this behaviour in the Windows and Linux clients at build time using a CMAKE flag. The default behaviour is kept the same, and nothing changes for the user unless the flag is manually set in the config file. A setter is included in this patch, although it is not yet used in the GUI. Signed-off-by: Micke Nordin --- doc/conffile.rst | 2 ++ src/gui/wizard/owncloudsetuppage.cpp | 2 ++ src/gui/wizard/owncloudsetuppage.h | 3 +++ src/gui/wizard/owncloudwizard.cpp | 15 +++++++++++---- src/gui/wizard/owncloudwizard.h | 4 ++++ src/libsync/configfile.cpp | 9 +++++++++ src/libsync/configfile.h | 4 ++++ src/libsync/networkjobs.cpp | 13 ++++++++++++- src/libsync/networkjobs.h | 1 + 9 files changed, 48 insertions(+), 5 deletions(-) diff --git a/doc/conffile.rst b/doc/conffile.rst index 8d403df30bd8a..670cc4d607540 100644 --- a/doc/conffile.rst +++ b/doc/conffile.rst @@ -41,6 +41,8 @@ Some interesting values that can be set on the configuration file are: | ``chunkSize`` | ``10000000`` (10 MB) | Specifies the chunk size of uploaded files in bytes. | | | | The client will dynamically adjust this size within the maximum and minimum bounds (see below). | +----------------------------------+--------------------------+--------------------------------------------------------------------------------------------------------+ +| ``forceLoginV2`` | ``false`` | If the client should force the new login flow, eventhough some circumstances might need the old flow. | ++----------------------------------+--------------------------+--------------------------------------------------------------------------------------------------------+ | ``minChunkSize`` | ``1000000`` (1 MB) | Specifies the minimum chunk size of uploaded files in bytes. | +----------------------------------+--------------------------+--------------------------------------------------------------------------------------------------------+ | ``maxChunkSize`` | ``1000000000`` (1000 MB) | Specifies the maximum chunk size of uploaded files in bytes. | diff --git a/src/gui/wizard/owncloudsetuppage.cpp b/src/gui/wizard/owncloudsetuppage.cpp index c7134478ab0ae..15be3e342dd5e 100644 --- a/src/gui/wizard/owncloudsetuppage.cpp +++ b/src/gui/wizard/owncloudsetuppage.cpp @@ -213,6 +213,8 @@ int OwncloudSetupPage::nextId() const return WizardCommon::Page_Flow2AuthCreds; #ifdef WITH_WEBENGINE case DetermineAuthTypeJob::WebViewFlow: + if (this->useFlow2) + return WizardCommon::Page_Flow2AuthCreds; return WizardCommon::Page_WebView; #endif // WITH_WEBENGINE case DetermineAuthTypeJob::NoAuthType: diff --git a/src/gui/wizard/owncloudsetuppage.h b/src/gui/wizard/owncloudsetuppage.h index e9c5da0d2c0d0..06cea0753695e 100644 --- a/src/gui/wizard/owncloudsetuppage.h +++ b/src/gui/wizard/owncloudsetuppage.h @@ -89,6 +89,9 @@ protected slots: QProgressIndicator *_progressIndi; OwncloudWizard *_ocWizard; AddCertificateDialog *addCertDial = nullptr; + + // Grab the forceLoginV2-setting from the wizard + bool useFlow2 = _ocWizard->useFlow2(); }; } // namespace OCC diff --git a/src/gui/wizard/owncloudwizard.cpp b/src/gui/wizard/owncloudwizard.cpp index faac2f5ad8934..a70421a762ebc 100644 --- a/src/gui/wizard/owncloudwizard.cpp +++ b/src/gui/wizard/owncloudwizard.cpp @@ -65,7 +65,8 @@ OwncloudWizard::OwncloudWizard(QWidget *parent) setPage(WizardCommon::Page_Flow2AuthCreds, _flow2CredsPage); setPage(WizardCommon::Page_AdvancedSetup, _advancedSetupPage); #ifdef WITH_WEBENGINE - setPage(WizardCommon::Page_WebView, _webViewPage); + if(!useFlow2()) + setPage(WizardCommon::Page_WebView, _webViewPage); #endif // WITH_WEBENGINE connect(this, &QDialog::finished, this, &OwncloudWizard::basicSetupFinished); @@ -78,7 +79,8 @@ OwncloudWizard::OwncloudWizard(QWidget *parent) connect(_httpCredsPage, &OwncloudHttpCredsPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); connect(_flow2CredsPage, &Flow2AuthCredsPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); #ifdef WITH_WEBENGINE - connect(_webViewPage, &WebViewPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); + if(!useFlow2()) + connect(_webViewPage, &WebViewPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); #endif // WITH_WEBENGINE connect(_advancedSetupPage, &OwncloudAdvancedSetupPage::createLocalAndRemoteFolders, this, &OwncloudWizard::createLocalAndRemoteFolders); @@ -235,7 +237,8 @@ void OwncloudWizard::successfulStep() #ifdef WITH_WEBENGINE case WizardCommon::Page_WebView: - _webViewPage->setConnected(); + if(!this->useFlow2()) + _webViewPage->setConnected(); break; #endif // WITH_WEBENGINE @@ -276,7 +279,11 @@ void OwncloudWizard::setAuthType(DetermineAuthTypeJob::AuthType type) _credentialsPage = _flow2CredsPage; #ifdef WITH_WEBENGINE } else if (type == DetermineAuthTypeJob::WebViewFlow) { - _credentialsPage = _webViewPage; + if(this->useFlow2()) { + _credentialsPage = _flow2CredsPage; + } else { + _credentialsPage = _webViewPage; + } #endif // WITH_WEBENGINE } else { // try Basic auth even for "Unknown" _credentialsPage = _httpCredsPage; diff --git a/src/gui/wizard/owncloudwizard.h b/src/gui/wizard/owncloudwizard.h index 58d3e915d9d1b..7b006c7272c25 100644 --- a/src/gui/wizard/owncloudwizard.h +++ b/src/gui/wizard/owncloudwizard.h @@ -21,6 +21,7 @@ #include #include +#include "libsync/configfile.h" #include "networkjobs.h" #include "wizard/owncloudwizardcommon.h" #include "accountfwd.h" @@ -87,6 +88,7 @@ class OwncloudWizard : public QWizard QSslKey _clientSslKey; // key extracted from pkcs12 QSslCertificate _clientSslCertificate; // cert extracted from pkcs12 QList _clientSslCaCertificates; + bool useFlow2() { return _useFlow2; }; public slots: void setAuthType(OCC::DetermineAuthTypeJob::AuthType type); @@ -131,6 +133,8 @@ public slots: bool _registration = false; + bool _useFlow2 = ConfigFile().forceLoginV2(); + friend class OwncloudSetupWizard; }; diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 60cb2cc71aeba..39949683f8bce 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -104,6 +104,8 @@ static constexpr char stopSyncingExistingFoldersOverLimitC[] = "stopSyncingExist static constexpr char confirmExternalStorageC[] = "confirmExternalStorage"; static constexpr char moveToTrashC[] = "moveToTrash"; +static constexpr char forceLoginV2C[] = "forceLoginV2"; + static constexpr char certPath[] = "http_certificatePath"; static constexpr char certPasswd[] = "http_certificatePasswd"; @@ -991,6 +993,13 @@ void ConfigFile::setMoveToTrash(bool isChecked) setValue(moveToTrashC, isChecked); } +bool ConfigFile::forceLoginV2() const { + return getValue(forceLoginV2C, QString(), false).toBool(); +} +void ConfigFile::setForceLoginV2(bool isChecked) { + setValue(forceLoginV2C, isChecked); +} + bool ConfigFile::showMainDialogAsNormalWindow() const { return getValue(showMainDialogAsNormalWindowC, {}, false).toBool(); } diff --git a/src/libsync/configfile.h b/src/libsync/configfile.h index 21e58412afcc5..0ca15b807c73c 100644 --- a/src/libsync/configfile.h +++ b/src/libsync/configfile.h @@ -153,6 +153,10 @@ class OWNCLOUDSYNC_EXPORT ConfigFile [[nodiscard]] bool moveToTrash() const; void setMoveToTrash(bool); + /** If we should force loginflow v2 */ + [[nodiscard]] bool forceLoginV2() const; + void setForceLoginV2(bool); + [[nodiscard]] bool showMainDialogAsNormalWindow() const; static bool setConfDir(const QString &value); diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp index 3c784675feb61..48f10eb363be3 100644 --- a/src/libsync/networkjobs.cpp +++ b/src/libsync/networkjobs.cpp @@ -44,6 +44,7 @@ #include "creds/abstractcredentials.h" #include "creds/httpcredentials.h" +#include "configfile.h" namespace OCC { @@ -1011,7 +1012,9 @@ bool JsonApiJob::finished() DetermineAuthTypeJob::DetermineAuthTypeJob(AccountPtr account, QObject *parent) : QObject(parent) , _account(account) + { + useFlow2 = ConfigFile().forceLoginV2(); } void DetermineAuthTypeJob::start() @@ -1077,7 +1080,11 @@ void DetermineAuthTypeJob::start() if (flow != QJsonValue::Undefined) { if (flow.toInt() == 1) { #ifdef WITH_WEBENGINE - _resultOldFlow = WebViewFlow; + if(!this->useFlow2) { + _resultOldFlow = WebViewFlow; + } else { + qCWarning(lcDetermineAuthTypeJob) << "Server only supports flow1, but this client was configured to only use flow2"; + } #else // WITH_WEBENGINE qCWarning(lcDetermineAuthTypeJob) << "Server does only support flow1, but this client was compiled without support for flow1"; #endif // WITH_WEBENGINE @@ -1111,6 +1118,8 @@ void DetermineAuthTypeJob::checkAllDone() // WebViewFlow > Basic if (_account->serverVersionInt() >= Account::makeServerVersion(12, 0, 0)) { result = WebViewFlow; + if(useFlow2) + result = LoginFlowV2; } #endif // WITH_WEBENGINE @@ -1123,6 +1132,8 @@ void DetermineAuthTypeJob::checkAllDone() // If we determined that we need the webview flow (GS for example) then we switch to that if (_resultOldFlow == WebViewFlow) { result = WebViewFlow; + if(useFlow2) + result = LoginFlowV2; } #endif // WITH_WEBENGINE diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h index b299bdb0aa387..c1337f1cc4002 100644 --- a/src/libsync/networkjobs.h +++ b/src/libsync/networkjobs.h @@ -549,6 +549,7 @@ class OWNCLOUDSYNC_EXPORT DetermineAuthTypeJob : public QObject bool _getDone = false; bool _propfindDone = false; bool _oldFlowDone = false; + bool useFlow2 = false; }; /** From 42012a0efb6483f7f49039c314ce48f317cb5e68 Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Wed, 6 Dec 2023 16:22:25 +0100 Subject: [PATCH 2/3] Apply suggestions from code review Formatting fixes Co-authored-by: Claudio Cambra Signed-off-by: Micke Nordin --- src/gui/wizard/owncloudsetuppage.cpp | 5 +++-- src/gui/wizard/owncloudwizard.cpp | 19 +++++++++++-------- src/libsync/configfile.cpp | 7 +++++-- src/libsync/networkjobs.cpp | 23 ++++++++++++----------- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/gui/wizard/owncloudsetuppage.cpp b/src/gui/wizard/owncloudsetuppage.cpp index 15be3e342dd5e..a5374eb59fc3c 100644 --- a/src/gui/wizard/owncloudsetuppage.cpp +++ b/src/gui/wizard/owncloudsetuppage.cpp @@ -213,8 +213,9 @@ int OwncloudSetupPage::nextId() const return WizardCommon::Page_Flow2AuthCreds; #ifdef WITH_WEBENGINE case DetermineAuthTypeJob::WebViewFlow: - if (this->useFlow2) - return WizardCommon::Page_Flow2AuthCreds; + if (this->useFlow2) { + return WizardCommon::Page_Flow2AuthCreds; + } return WizardCommon::Page_WebView; #endif // WITH_WEBENGINE case DetermineAuthTypeJob::NoAuthType: diff --git a/src/gui/wizard/owncloudwizard.cpp b/src/gui/wizard/owncloudwizard.cpp index a70421a762ebc..37b3ebeea4fc1 100644 --- a/src/gui/wizard/owncloudwizard.cpp +++ b/src/gui/wizard/owncloudwizard.cpp @@ -65,8 +65,9 @@ OwncloudWizard::OwncloudWizard(QWidget *parent) setPage(WizardCommon::Page_Flow2AuthCreds, _flow2CredsPage); setPage(WizardCommon::Page_AdvancedSetup, _advancedSetupPage); #ifdef WITH_WEBENGINE - if(!useFlow2()) - setPage(WizardCommon::Page_WebView, _webViewPage); + if (!useFlow2()) { + setPage(WizardCommon::Page_WebView, _webViewPage); + } #endif // WITH_WEBENGINE connect(this, &QDialog::finished, this, &OwncloudWizard::basicSetupFinished); @@ -79,8 +80,9 @@ OwncloudWizard::OwncloudWizard(QWidget *parent) connect(_httpCredsPage, &OwncloudHttpCredsPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); connect(_flow2CredsPage, &Flow2AuthCredsPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); #ifdef WITH_WEBENGINE - if(!useFlow2()) - connect(_webViewPage, &WebViewPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); + if (!useFlow2()) { + connect(_webViewPage, &WebViewPage::connectToOCUrl, this, &OwncloudWizard::connectToOCUrl); + } #endif // WITH_WEBENGINE connect(_advancedSetupPage, &OwncloudAdvancedSetupPage::createLocalAndRemoteFolders, this, &OwncloudWizard::createLocalAndRemoteFolders); @@ -237,8 +239,9 @@ void OwncloudWizard::successfulStep() #ifdef WITH_WEBENGINE case WizardCommon::Page_WebView: - if(!this->useFlow2()) - _webViewPage->setConnected(); + if (!this->useFlow2()) { + _webViewPage->setConnected(); + } break; #endif // WITH_WEBENGINE @@ -280,9 +283,9 @@ void OwncloudWizard::setAuthType(DetermineAuthTypeJob::AuthType type) #ifdef WITH_WEBENGINE } else if (type == DetermineAuthTypeJob::WebViewFlow) { if(this->useFlow2()) { - _credentialsPage = _flow2CredsPage; + _credentialsPage = _flow2CredsPage; } else { - _credentialsPage = _webViewPage; + _credentialsPage = _webViewPage; } #endif // WITH_WEBENGINE } else { // try Basic auth even for "Unknown" diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 39949683f8bce..7d2ebb4f39369 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -993,10 +993,13 @@ void ConfigFile::setMoveToTrash(bool isChecked) setValue(moveToTrashC, isChecked); } -bool ConfigFile::forceLoginV2() const { +bool ConfigFile::forceLoginV2() const +{ return getValue(forceLoginV2C, QString(), false).toBool(); } -void ConfigFile::setForceLoginV2(bool isChecked) { + +void ConfigFile::setForceLoginV2(bool isChecked) +{ setValue(forceLoginV2C, isChecked); } diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp index 48f10eb363be3..93e220c478b5a 100644 --- a/src/libsync/networkjobs.cpp +++ b/src/libsync/networkjobs.cpp @@ -1012,9 +1012,8 @@ bool JsonApiJob::finished() DetermineAuthTypeJob::DetermineAuthTypeJob(AccountPtr account, QObject *parent) : QObject(parent) , _account(account) - { - useFlow2 = ConfigFile().forceLoginV2(); + useFlow2 = ConfigFile().forceLoginV2(); } void DetermineAuthTypeJob::start() @@ -1080,11 +1079,11 @@ void DetermineAuthTypeJob::start() if (flow != QJsonValue::Undefined) { if (flow.toInt() == 1) { #ifdef WITH_WEBENGINE - if(!this->useFlow2) { - _resultOldFlow = WebViewFlow; - } else { - qCWarning(lcDetermineAuthTypeJob) << "Server only supports flow1, but this client was configured to only use flow2"; - } + if(!this->useFlow2) { + _resultOldFlow = WebViewFlow; + } else { + qCWarning(lcDetermineAuthTypeJob) << "Server only supports flow1, but this client was configured to only use flow2"; + } #else // WITH_WEBENGINE qCWarning(lcDetermineAuthTypeJob) << "Server does only support flow1, but this client was compiled without support for flow1"; #endif // WITH_WEBENGINE @@ -1118,8 +1117,9 @@ void DetermineAuthTypeJob::checkAllDone() // WebViewFlow > Basic if (_account->serverVersionInt() >= Account::makeServerVersion(12, 0, 0)) { result = WebViewFlow; - if(useFlow2) - result = LoginFlowV2; + if (useFlow2) { + result = LoginFlowV2; + } } #endif // WITH_WEBENGINE @@ -1132,8 +1132,9 @@ void DetermineAuthTypeJob::checkAllDone() // If we determined that we need the webview flow (GS for example) then we switch to that if (_resultOldFlow == WebViewFlow) { result = WebViewFlow; - if(useFlow2) - result = LoginFlowV2; + if (useFlow2) { + result = LoginFlowV2; + } } #endif // WITH_WEBENGINE From 6ca4ace09fa3d02ef3bb1b33b669c42eb930c9f9 Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Wed, 6 Dec 2023 16:46:31 +0100 Subject: [PATCH 3/3] OwncloudWizard: Move implementation to cpp-file Signed-off-by: Micke Nordin --- src/gui/wizard/owncloudwizard.cpp | 5 +++++ src/gui/wizard/owncloudwizard.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/gui/wizard/owncloudwizard.cpp b/src/gui/wizard/owncloudwizard.cpp index 37b3ebeea4fc1..238286aa0accd 100644 --- a/src/gui/wizard/owncloudwizard.cpp +++ b/src/gui/wizard/owncloudwizard.cpp @@ -193,6 +193,11 @@ QStringList OwncloudWizard::selectiveSyncBlacklist() const return _advancedSetupPage->selectiveSyncBlacklist(); } +bool OwncloudWizard::useFlow2() const +{ + return _useFlow2; +} + bool OwncloudWizard::useVirtualFileSync() const { return _advancedSetupPage->useVirtualFileSync(); diff --git a/src/gui/wizard/owncloudwizard.h b/src/gui/wizard/owncloudwizard.h index 7b006c7272c25..7f69587a45da7 100644 --- a/src/gui/wizard/owncloudwizard.h +++ b/src/gui/wizard/owncloudwizard.h @@ -65,6 +65,7 @@ class OwncloudWizard : public QWizard [[nodiscard]] QString ocUrl() const; [[nodiscard]] QString localFolder() const; [[nodiscard]] QStringList selectiveSyncBlacklist() const; + [[nodiscard]] bool useFlow2() const; [[nodiscard]] bool useVirtualFileSync() const; [[nodiscard]] bool isConfirmBigFolderChecked() const; @@ -88,7 +89,6 @@ class OwncloudWizard : public QWizard QSslKey _clientSslKey; // key extracted from pkcs12 QSslCertificate _clientSslCertificate; // cert extracted from pkcs12 QList _clientSslCaCertificates; - bool useFlow2() { return _useFlow2; }; public slots: void setAuthType(OCC::DetermineAuthTypeJob::AuthType type);